Skip to content

Conversation

@diegoreymendez
Copy link
Contributor

Description:

Adds nullability information to Blog.h, in order to make sure that we're correctly treating access to its properties and methods.

Details:

When accessing Objective-C properties, parameters and return values that don't have nullability information available from Swift code, Xcode assumes those methods and properties to be IUOs. In fact we can treat them as both optionals or non-optionals and Xcode will not give any warning to us about their usage.

This is dangerous, and this PR aims to provide Xcode with more information so that it will treat these properties, params and return values as either optionals or non-optionals.

What's inside the scope of this PR:

  • Adding appropriate nullability information for all methods and properties in Blog.h.
  • Reviewing our previous code to make any force-unwrapping explicit, thus raising awareness of spots were we need to improve our code.
  • Reviewers should make sure there are no logic changes in the code. At all.

What's NOT inside the scope of this PR:

  • Fixing the now-explicit force-unwrap calls with proper unwrapping: there are several reason why I'm leaving this out of this PR purposedly:
    • This PR is not really adding those force-unwrap calls, just making them explicit.
    • The amount of force-unwrap calls make it risky to try and fix everything in this PR. The new code is no worse than the old code... in fact it's better since now we know we're force-unwrapping.
    • The fix may not be the same for all the cases we have. We may find mechanisms to make some of these properties and methods non-optional, and some others will need regular unwrapping. Handling each case atomically is the best option.

Changes:

  • Added nullability info to Blog.h. All properties were marked nullable, while methods were analyzed case-by-case.
  • Added explicit force-unwrapping were necessary.

Test:

  • Make sure that there are no logic changes in the code.
  • Make sure all previously implicit force-unwraps are now explicit.
  • Build the app, make sure it builds and run.
  • Run the unit tests.

Needs review: @SergioEstevao

@koke
Copy link
Member

koke commented May 9, 2016

Previous attempt for reference: #4762

@SergioEstevao
Copy link
Contributor

:shipit:

@diegoreymendez diegoreymendez merged commit 02a6538 into develop May 11, 2016
@diegoreymendez diegoreymendez deleted the cleanup/add-nullability-to-blog-2 branch May 11, 2016 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants