application-services icon indicating copy to clipboard operation
application-services copied to clipboard

Bug 1893086 - Port the potential impressions framework to the suggest component

Open 0c0w3 opened this issue 1 year ago • 0 comments

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1893086 for details. Notes:

  • I'm not sure who to request review from these days, so please forward the request if you'd like!
  • This leaves a keyword-matching strategy as a to-do. I think we'll want to use fts5 but I'm not sure. I'd like to do that as a follow-up.
  • This modifies SuggestionQuery, which breaks most existing tests in store.rs. To prevent that from happening again in the future as much as possible, I added a SuggestionQueryBuilder and a couple macros.
  • SuggestionQueryBuilder isn't exposed to consumers. Should it be? I tried adding it to the udl but that requires Arc-ifying it, which makes it a pain to use in tests (I think?). Can we do that later if it turns out to be helpful?
  • Since this does modify SuggestionQuery, do I need to do something to avoid breaking mobile? I read the breaking changes info but tbh I don't understand it.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • [ ] This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • [ ] Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • [ ] Tests: This PR includes thorough tests or an explanation of why it does not
  • [ ] Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • [ ] Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.

Branch builds: add [firefox-android: branch-name] to the PR title.

0c0w3 avatar Apr 24 '24 03:04 0c0w3