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

Bug 1923689 - Implement basic configuration processing on SearchEngineSelector.

Open Standard8 opened this issue 1 year ago • 1 comments

This implements processing the main parts of the search configuration JSON into the Rust structure using serde_json. Currently the processing applies to:

  • The identifier and base properties of the engine records.
  • The default engine records.

As variant and environment handling is not yet implemented, the filter function will return all engines defined in the configuration.

These functions are not currently used, hence no breaking API changes, similarly with change log.

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • [X] 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
  • [X] 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.
  • [X] Tests: This PR includes thorough tests or an explanation of why it does not
  • [X] 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
  • [X] Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

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

Standard8 avatar Oct 09 '24 17:10 Standard8

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 49.30%. Comparing base (9e9705c) to head (71a936a). Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6411      +/-   ##
==========================================
- Coverage   51.78%   49.30%   -2.48%     
==========================================
  Files         132      146      +14     
  Lines       13053    13708     +655     
==========================================
  Hits         6759     6759              
- Misses       6294     6949     +655     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 09 '24 17:10 codecov-commenter

@linabutler Thank you for all the excellent comments. I definitely like the idea of cloning up-front. Although it feels like cloning a large amount when we might use just a small amount, I agree it is better that way, so that we can reduce the individual allocations & make the code simpler.

Standard8 avatar Nov 08 '24 15:11 Standard8