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

NTP: Remove option to bookmark top sites #5490

Merged
merged 1 commit into from
May 11, 2020
Merged

NTP: Remove option to bookmark top sites #5490

merged 1 commit into from
May 11, 2020

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented May 7, 2020 •
edited
Loading

@cezaraugusto cezaraugusto self-assigned this May 7, 2020
bsclifton
bsclifton approved these changes May 7, 2020
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Tested it out- this works great 😄 Nice!

@bsclifton
Copy link
Member

CI failed on iOS, but good on other platforms. Will restart for iOS. There was an intermittent error (not a blocker) captured with brave/brave-browser#5397 for the rewards test on macOS:

12:40:49  [556/556] BraveRewardsBrowserTest.ProcessPendingContributions (37762 ms)
12:40:49  1 test failed:
12:40:49      BraveRewardsBrowserTest.ProcessPendingContributions (../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:2206)

Once iOS looks good, this will be ready for merge 👍

@bsclifton bsclifton added CI/skip-android Do not run CI builds for Android CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels May 7, 2020
@bsclifton bsclifton added this to the 1.10.x - Nightly milestone May 7, 2020
@bsclifton
Copy link
Member

bsclifton commented May 11, 2020 •
edited
Loading

Rebased & pushed; let's see how CI does😄

@AndriusA AndriusA mentioned this pull request May 11, 2020
Closed
32 tasks
@bsclifton
Copy link
Member

Everything is green via Travis CI and the child CI job (https://ci.brave.com/job/pr-brave-browser-ca-9481/4/) - should be good to merge! 😄

The parent job (https://ci.brave.com/job/brave-core-build-pr/job/PR-5490/5/display/redirect) has the No such property problem, but @mihaiplesa fixed this last nite

@bsclifton bsclifton merged commit 3d89940 into master May 11, 2020
@bsclifton bsclifton deleted the ca-9481 branch May 11, 2020 16:44
brave-builds pushed a commit that referenced this pull request May 11, 2020
@brave-browser-releases
Uplift of #5490 (squashed) to beta
53ceabf
@brave-builds brave-builds mentioned this pull request May 11, 2020
Merged
4 tasks
@NejcZdovc NejcZdovc mentioned this pull request May 11, 2020
Merged
4 tasks
This was referenced May 12, 2020
Closed
Closed
bsclifton pushed a commit that referenced this pull request May 26, 2020
@brave-browser-releases @bsclifton
Uplift of #5490 (squashed) to beta
46ce56a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@AndriusA AndriusA AndriusA approved these changes

@petemill petemill Awaiting requested review from petemill

@simonhong simonhong Awaiting requested review from simonhong

Assignees

@cezaraugusto cezaraugusto

Labels
CI/skip-android Do not run CI builds for Android CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Milestone
1.10.x - Release
Development

Successfully merging this pull request may close these issues.

Remove bookmark icon from top sites
3 participants
@cezaraugusto @bsclifton @AndriusA