-
Notifications
You must be signed in to change notification settings - Fork 892
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
Cleans up pre-populated search engines. #1648
Conversation
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.
Tested this PR on MacOS and works great. Also all manual tests works as expected.
@mkarolin we are also discussing keeping yahoo; let me cc you on the discussion in slack |
👍 confirmed these are the 5 we want to keep |
Thanks @mkarolin, could you do uplift PRs too? |
Engines kept: - Google - Duckduckgo - Qwant - Bing - Startpage. Adds a unit test to verify the search engines overridden by us are used instead of the original ones. Lint fixes. Fixes brave/brave-browser#3316
f579d34
to
aca1fdf
Compare
Did a rebase (sorry for invaliding review! in case you wanted to re-approve, @simonhong) |
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.
++
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.
For future PRs, it might be better to have two commits; one fixing the issue and the second fixing linting errors... but awesome work 😄👍
{"Google", TemplateURLPrepopulateData::google}, | ||
{"Infogalactic", TemplateURLPrepopulateData::infogalactic}, |
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.
🎉
Engines kept:
Adds a unit test to verify the search engines overridden by us are used
instead of the original ones.
Lint fixes.
Fixes brave/brave-browser#3316
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests
) ongit rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Automated testing:
npm run test -- brave_unit_tests --filter=BraveTemplateURLPrepopulateDataTest.*
npm run test -- brave_unit_tests --filter=BraveTemplateURLServiceUtilTest.*
Manual testing:
changed (e.g. Google for U.S., Qwant for DE & FR),
present and that the default engine has not changed.
are present and that the default engine is Amazon.
and that Qwant is the default engine for DE & FR and Google for all others.
remains the default engine.
default engine.
Reviewer Checklist: