Annotate getDataClass and getDataSource from DataFetcher as NonNull #2203
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
| @@ -67,11 +67,13 @@ public void cancel() { | |||
| } | |||
| } | |||
|
|
|||
| @android.support.annotation.NonNull | |||
There was a problem hiding this comment.
Can you import NonNull instead of referencing it via the package name?
| @@ -87,11 +87,13 @@ public void cancel() { | |||
| } | |||
| } | |||
|
|
|||
| @android.support.annotation.NonNull | |||
There was a problem hiding this comment.
Same here, can you import NonNull?
| @@ -89,11 +89,13 @@ public void cancel() { | |||
| // TODO: call cancel on the client when this method is called on a background thread. See #257 | |||
| } | |||
|
|
|||
| @android.support.annotation.NonNull | |||
There was a problem hiding this comment.
And here too, can you import NonNull?
There was a problem hiding this comment.
@sjudd Actually this is strange that the compilation and test passed, the refactoring was made by AS.
Integration libraries does not import Support Annotation libraries.
To import the NonNull I need to add the dependency is it ok for you or drop the annotation in those ?
There was a problem hiding this comment.
Yeah it's ok to add the dependency, the main library requires the annotations anyway so it's not a new dependency for applications.
Returning null from those will trigger obscure crash in MultiClassKey, annotation helps preventing that.
|
CLAs look good, thanks! |
|
Pr updated. |
|
Great thanks! |
Description
Annotate getDataClass and getDataSource from DataFetcher as NonNull
Returning null from those will trigger obscure crash in MultiClassKey, annotation helps preventing that.
Motivation and Context
Help devs to avoid pitfalls :)