[iOS] nullability annotations on all public headers #715
Conversation
Hi @damienpontifex, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, AZPRBOT; |
@@ -30,7 +30,7 @@ | |||
|
|||
|
|||
/// The user id of the end user. | |||
@property (nonatomic, copy, readonly) NSString *userId; | |||
@property (nonatomic, copy, readonly, nonnull) NSString *userId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be null, all log out does is set this to null iirc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I though logout just set currentUser to nil in MSClient
@@ -18,7 +18,7 @@ | |||
///@{ | |||
|
|||
/// Initializes an *MSUser* instance with the given user id. | |||
-(id)initWithUserId:(NSString *)userId; | |||
-(nonnull instancetype)initWithUserId:(nonnull NSString *)userId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a unit test: -(void) testInitWithUserIdAllowsNilUserId, that says you can use nil here. To me, since the auth token is the only important data it makes sense to allow no user Id (Its been awhile, but I don't remember caching my userIds in the past when building auth out)
@phvannor was there anymore changes/feedback on this? |
@damienpontifex I was out the last couple work days, I'll take a final pass today/tomorrow to refresh my memory, I think it was OK now. |
No worries, was just doing my Monday thing to review open items |
|
||
/// The item that will be sent to the server when execute is called. | ||
@property (nonatomic, strong) NSDictionary *item; | ||
@property (nonatomic, strong, nonnull) NSDictionary *item; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one should be nullable, the internal constructor can make one w/o an item, so it is not 100% always around.
👍 |
Looks like it can't merge, can you resolve any conflicts so I can take this? |
Merged changes. Added nullable annotations to the returned operations we added from #709 |
[iOS] nullability annotations on all public headers
Continued nullability annotations on all public headers for iOS SDK after #704