Nullability Annotations #83
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apbendi
force-pushed
the
nullability
branch
from
aac28ef
to
22ffc2e
Compare
spectorar
reviewed
Nov 14, 2016
| [self save:callback]; | ||
| } | ||
|
|
||
| - (void)saveWithUser:(CMUser *)user callback:(CMStoreObjectUploadCallback)callback; |
There was a problem hiding this comment.
It looks like the user param is being ignored here. Can you explain?
There was a problem hiding this comment.
Good eye @spectorar. This method is being deprecated in this release. You are correct in saying the user parameter is ignored. It was always ignored, in fact. It is only possible to save an object to the user who is currently logged in, so there is no need to pass in a user. The replacement method is simply saveAtUserLevel:
…associated callbacks
…er; properly return an *immutable* array of decoded objects
… immutable dictionary representation when encoding an object
…erly return an immutable dictionary of parameters
…associated protocols
|
I've rebased this PR to the current release and upgraded it to the latest version of CocoaPods. After chatting with @jraymondcm, I think I want to fix #70 and push that as another bugfix on 1.7, so we should hold off on merging this for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds nullability annotations to the entire public interface of the SDK. In almost all cases, I've simply annotated the current behavior by reading and testing the code, rather than making a behavior change to make a parameter work one way or the other. In future releases, we should definitely consider making internal behavior changes to make some nullable parameters nonnull. (An obvious case here is collections. Why return nil when you can just return an empty array?).
This change is helpful in Objective-C (the compiler will emit warnings if you blatantly ignore the annotations) but is a huge change for developers using Swift.
In general, the change will make using the SDK in Swift feel more natural, and will definitely make it more safe, by leveraging Swift's optionals. This is not the be-all end-all for Swift compatibility, by any means. There are lots of places where we could still improve: generics, naming, initialization, and even by changing some of the newly annotated nullable parameters to nonnull, and vice versa. But by at least annotating the current state of the SDK around nullability, we take a huge step toward making the SDK more Swifty.
For existing Swift projects, there will technically be some breaking changes. These mostly revolve around the way existing developers are handling implicitly unwrapped optionals. For example, if a developer is unwrapping a parameter in their code that is now marked nonnull, after updating the compiler will tell them they don't need to unwrap it. This may make for a slightly annoying migration for some developers, but should actually result in cleaner, safer code when they are done.
It's also important to set realistic expectations about this release. The SDK has a large surface area and was first written long before Swift or the concept of nullability annotations existed. As such, we should expect some rough spots, where our annotations are inconvenient or wrong, and we hear feedback from developers as such. We should be prepared to react to this feedback and release point updates quickly. Teams using Swift should be instructed to do careful testing of their app after upgrading to this version. Developers experiencing issues can always opt to keep their project on version 1.7.x until patches are released.
Overall, though, these annotations represent a big chunk of the work needed to modernize the SDK and adapt it for Swift.