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

fix NullPointerException #733

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix NullPointerException #733

wants to merge 1 commit into from

Conversation

tyf1946
Copy link

@tyf1946 tyf1946 commented Jun 15, 2019

HomeActivity method onPrepareOptionsMenu
menu.findItem(R.id.menu_designer_news_login) return null object
use onCreateOptionsMenu find menu_designer_news_login

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I ran ./gradlew spotlessApply before submitting the PR
  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

📸 Screenshots / GIFs

HomeActivity method onPrepareOptionsMenu
menu.findItem(R.id.menu_designer_news_login) return null object
use onCreateOptionsMenu find menu_designer_news_login
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@keyboardsurfer
Copy link
Collaborator

Hi and thanks for your contribution.
Before we can continue with this, please make sure both checks go through.

@@ -100,6 +100,7 @@ class HomeActivity : AppCompatActivity() {
private lateinit var loading: ProgressBar
private lateinit var feedAdapter: FeedAdapter
private lateinit var filtersList: RecyclerView
private lateinit var mDesignerNewsLogin: MenuItem
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't use m prefix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also I recommend not to hold on to the item in the activity scope.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for your suggestion

@@ -450,6 +450,12 @@ class HomeActivity : AppCompatActivity() {
return true
}

override fun onCreateOptionsMenu(menu: Menu?): Boolean {
menuInflater.inflate(R.menu.main,menu)
mDesignerNewsLogin = menu!!.findItem(R.id.menu_designer_news_login)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't use !! where it can be avoided.
In this case, checking whether menu is not null is valid.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for your suggestion

@tyf1946
Copy link
Author

tyf1946 commented Jul 12, 2019

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@florina-muntenescu
Copy link
Collaborator

Run gradlew spotlessApply please

Copy link
Collaborator

@keyboardsurfer keyboardsurfer left a comment

Choose a reason for hiding this comment

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

Please address my review comments in order to proceed on this PR.

@codingjeremy codingjeremy changed the base branch from master to main September 29, 2020 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants