Skip to content
This repository was archived by the owner on Nov 1, 2022. It is now read-only.

Conversation

@pocmo
Copy link
Contributor

@pocmo pocmo commented Oct 13, 2020

This migrates samples-browser to stop using SearchEngineManager. For that I hide the provider of the default search engine behind an interface. This is a temporary solution until we have all apps using the SearchEngine from BrowserStore. After that we can get rid of that and make all our components use BrowserStore directly.

I tested this in Fenix too and have a matching patch locally. Once this lands we should be able to migrate Reference Browser too.

@pocmo pocmo added the 🕵️‍♀️ needs review PRs that need to be reviewed label Oct 13, 2020
@pocmo pocmo requested a review from csadilek October 13, 2020 15:13
@pocmo pocmo linked an issue Oct 13, 2020 that may be closed by this pull request
@csadilek csadilek self-assigned this Oct 13, 2020
@pocmo
Copy link
Contributor Author

pocmo commented Oct 13, 2020

I totally forgot the tests. 😅

Will update those tomorrow.

Copy link
Contributor

@csadilek csadilek left a comment

Choose a reason for hiding this comment

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

👍

Should we file a ticket to get rid of the temporary types and link to in source?

@pocmo
Copy link
Contributor Author

pocmo commented Oct 14, 2020

Should we file a ticket to get rid of the temporary types and link to in source?

👍 Filed #8686 and will add it to the code.

@pocmo pocmo force-pushed the sample-browser-search branch from c568e1c to b5f285d Compare October 14, 2020 10:04
…wserStore instead of SearchEngineManager.

This migrates sample-browser to stop using SearchEngineManager. For that I hide the provider of the default
search engine behind an interface. This is a temporary solution until we have all apps using the SearchEngine
from BrowserStore. After that we can get rid of that and make all our components use BrowserStore directly.

I tested this in Fenix too and have a matching patch locally. Once this lands we should be able to migrate
Reference Browser too.
@pocmo pocmo force-pushed the sample-browser-search branch from b5f285d to a037a2d Compare October 14, 2020 10:42
@pocmo pocmo added 🛬 needs landing PRs that are ready to land and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Oct 14, 2020
@mergify mergify bot merged commit 3ab790c into mozilla-mobile:master Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🛬 needs landing PRs that are ready to land

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate new search code into sample browser

2 participants