Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for Activity and Fragment coming from the support libraries: #275

Conversation

yanislavm
Copy link
Contributor

@yanislavm yanislavm commented Dec 18, 2018

  • Fragment and Activity classes that are coming from the support library and Android X
  • onCreateView and onAttach for the Fragment

Thank you for contributing to NullAway!

Please note that once you click "Create Pull Request" you will be asked to sign our Uber Contributor License Agreement via CLA assistant.

Before pressing the "Create Pull Request" button, please provide the following:

  • A description about what and why you are contributing, even if it's trivial.
    I noticed that NullAway is not recognising the onCreate method of the Activities and Fragments that are coming from the android support library (and AnddroidX library).
    Also for the Fragment it makes sense to support and onCreateView as well as onAttach as those are also like an entry point for the Fragment - in OnCreate we are creating the View and initialising the viewElements.

  • The issue number(s) or PR number(s) in the description if you are contributing in response to those.

  • If applicable, unit tests.

* Fragment and Activity classes that are coming from the support library and Android X
* onCreateView and onAttach for the Fragment
@CLAassistant
Copy link

CLAassistant commented Dec 18, 2018

CLA assistant check
All committers have signed the CLA.

@yanislavm yanislavm changed the title Adding support for: Adding support for Activity and Fragment coming from the support libraries: Dec 18, 2018
@msridhar
Copy link
Collaborator

This change looks great, thanks! Is there any way to add a test though? TBH I'm not sure about our existing coverage of these built-in initializers, but it'd be good to get a test going now at least for the new ones.

@yanislavm yanislavm force-pushed the feature/support-for-different-initializers-for-android branch from 4a71c49 to 9b3f7ab Compare December 21, 2018 14:34
Copy link
Collaborator

@msridhar msridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic! Once we address the comments we should be good to land. Please remove all the @NonNull annotations if possible.

Copy link
Collaborator

@msridhar msridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@msridhar msridhar merged commit ba56d5b into uber:master Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants