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

Issue #9801: Refactor AutofillCreditCardsAddressesStorage into its own :service-sync-autofill component #9848

Merged
merged 2 commits into from
Mar 10, 2021

Conversation

gabrielluong
Copy link
Member

Fixes #9801

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@gabrielluong gabrielluong requested review from grigoryk and a team as code owners March 9, 2021 05:46
@gabrielluong gabrielluong added the 🕵️‍♀️ needs review PRs that need to be reviewed label Mar 9, 2021
@gabrielluong gabrielluong changed the title Issue #9801: Refactor AutofillCreditCardsAddressesStorage into its own component Issue #9801: Refactor AutofillCreditCardsAddressesStorage into its own :service-sync-autofill component Mar 9, 2021
@gabrielluong gabrielluong added this to the 74.0.0 milestone Mar 9, 2021
Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

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

You need to add an entry to taskcluster/ci/config.yml as well when you add a new project in the .buildconfig.yml file.

@@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package mozilla.components.browser.storage.sync
package mozilla.components.service.sync.autofill
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the Gecko delegate live in the browser-engine-gecko-* components?

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't actually the delegate wrapper around GV's Autcomplete (yet to be landed yet). We would add something equivalent to GeckoLoginDelegateWrapper in browser-engine-gecko-*. This is equivalent to GeckoLoginStorageDelegate, but you could be right since this needs to be attached to the runtime.

I don't know enough about the specifics of where this should live, but if we determine both GeckoLoginStorageDelegate and this file should be in browser-engine-gecko-*, let's file a new issue and move them both there.

@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #9848 (e01663c) into master (4155fff) will decrease coverage by 5.82%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #9848      +/-   ##
============================================
- Coverage     74.05%   68.22%   -5.83%     
+ Complexity     5655      531    -5124     
============================================
  Files           772       76     -696     
  Lines         28766     3163   -25603     
  Branches       4738      500    -4238     
============================================
- Hits          21303     2158   -19145     
+ Misses         4986      734    -4252     
+ Partials       2477      271    -2206     
Impacted Files Coverage Δ Complexity Δ
...a/mozilla/components/browser/storage/sync/Types.kt 80.43% <ø> (-10.00%) 0.00 <0.00> (ø)
...nc/autofill/AutofillCreditCardsAddressesStorage.kt 61.90% <ø> (ø) 12.00 <0.00> (?)
...tofill/GeckoCreditCardsAddressesStorageDelegate.kt 12.50% <ø> (ø) 3.00 <0.00> (?)
.../mozilla/components/service/sync/autofill/Types.kt 100.00% <100.00%> (ø) 0.00 <0.00> (?)
...mponents/feature/share/adapter/RecentAppAdapter.kt
...ain/java/mozilla/components/service/glean/Glean.kt
...oncept/engine/manifest/parser/ShareTargetParser.kt
...feature/recentlyclosed/RecentlyClosedMiddleware.kt
...s/browser/icons/pipeline/IconResourceComparator.kt
...wser/menu2/adapter/SmallMenuCandidateViewHolder.kt
... and 693 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4155fff...e01663c. Read the comment docs.

…ge into its own :service-sync-autofill component
@grigoryk grigoryk added 🛬 needs landing PRs that are ready to land and removed 🕵️‍♀️ needs review PRs that need to be reviewed labels Mar 10, 2021
@mergify mergify bot merged commit 5ab780f into mozilla-mobile:master Mar 10, 2021
@gabrielluong gabrielluong removed this from the 74.0.0 milestone Mar 12, 2021
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.

Refactor AutofillCreditCardsAddressesStorage into its own component
3 participants