-
Notifications
You must be signed in to change notification settings - Fork 885
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
Adds initialized to panel and NTP flow #6321
Conversation
445bb09
to
f6d090f
Compare
f6d090f
to
e3e66dd
Compare
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.
LGTM
e3a4bed
to
dd5c0ba
Compare
state.initializing = true | ||
|
||
state[key] = enable | ||
chrome.braveRewards.saveSetting(key, enable ? '1' : '0') |
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.
Elsewhere in our code, I see us calling chrome.send('brave_rewards.saveSetting', [key, enable.toString()])
. Might be good to be consistent, unless those are two different use cases?
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.
yeah tried to do toString, but was failing, will try again
@NejcZdovc and I had a call where we looked at PR build from https://bravesoftware.slack.com/archives/CMH8DU4TF/p1596542545348200 Generally we checked:
We found 3 issues:
@NejcZdovc if I missed anything please feel free to add 😄 |
45d2c4d
to
533c430
Compare
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.
LGTM
533c430
to
dae1ca4
Compare
CI failed on unrelated browser test |
Verified using
For brave/brave-browser#11038:
For brave/brave-browser#11039:
Since brave/brave-browser#10783 is labeled Additionally did the following checks:
|
Resolves brave/brave-browser#11038
Resolves brave/brave-browser#11039
Resolves brave/brave-browser#10783
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
plan 1:
plan 2:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.