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.

Sign up for GitHub

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

Jump to bottom

Fixes no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119 #3059

Merged
merged 1 commit into from
Aug 2, 2019
Merged

Fixes no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119 #3059

merged 1 commit into from
Aug 2, 2019

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jul 31, 2019 •
edited
Loading

Fixes brave/brave-browser#5434

Submitter Checklist:

Test Plan:

  • Confirm upgrade paths work as expected from 0.62.x to 0.67.x
  • Confirm upgrade paths work as expected from 0.63.x to 0.67.x
  • Confirm upgrade paths work as expected from 0.66.x to 0.67.x

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@tmancey tmancey added this to the 0.70.x - Nightly milestone Jul 31, 2019
@tmancey tmancey self-assigned this Jul 31, 2019
@tmancey tmancey force-pushed the issues/5434 branch 2 times, most recently from 7d3630d to 0479c27 Compare July 31, 2019 09:52
@tmancey tmancey requested a review from NejcZdovc July 31, 2019 09:53
@tmancey tmancey force-pushed the issues/5434 branch 3 times, most recently from a527b3f to 6716ae5 Compare July 31, 2019 11:53
NejcZdovc
NejcZdovc previously approved these changes Jul 31, 2019
View reviewed changes
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment •
edited
Loading

Choose a reason for hiding this comment

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

code looks good. Testing it e2e will be done by @tmancey and QA after merge on nightly. Please wait for CI to complete.

NejcZdovc
NejcZdovc previously approved these changes Jul 31, 2019
View reviewed changes
@tmancey tmancey requested a review from emerick July 31, 2019 15:03
emerick
emerick previously approved these changes Jul 31, 2019
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@tmancey tmancey force-pushed the issues/5434 branch 7 times, most recently from 03732ab to 762b0e5 Compare August 1, 2019 17:45
@tmancey tmancey merged commit e3154c5 into master Aug 2, 2019
@tmancey tmancey deleted the issues/5434 branch August 2, 2019 11:17
@tmancey tmancey mentioned this pull request Aug 4, 2019
Merged
32 tasks
tmancey added a commit that referenced this pull request Aug 5, 2019
@tmancey
Follow up to there is no ads enable alert, ads are enabled by default…
d1ce246 
… when upgrading build from 0.62.51

Fixes brave/brave-browser#5531
Requires #3059
@LaurenWags LaurenWags mentioned this pull request Oct 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@bridiver bridiver Awaiting requested review from bridiver

@ryanml ryanml Awaiting requested review from ryanml

@NejcZdovc NejcZdovc Awaiting requested review from NejcZdovc

Assignees

@tmancey tmancey

Labels
bug feature/ads
Projects
None yet
Milestone
0.69.x - Release
Development

Successfully merging this pull request may close these issues.

There is no ads enable alert, ads are enabled by default when upgrading build from 0.62.51 to 0.67.119
3 participants
@tmancey @emerick @NejcZdovc