Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Move settings in components #12675

Merged
merged 3 commits into from
Jul 21, 2020
Merged

Conversation

NotWoods
Copy link
Contributor

Summary

We set up our dependencies inside the Components class, except for Settings. This PR moves the Settings class into Components.

Motivation

The Settings class is a singleton that isn't easily mocked. Tests involving settings vary from using static mocks, overriding the singleton, and manipulating the internal SharedPreferences. Moving it into Components makes it easier to write tests by just using normal mocks.

Settings is probably a singleton just for historic reasons. A Settings singleton existed in Focus long before we had any components, and from there it made it to Fenix.

@NotWoods NotWoods force-pushed the settings-component branch 4 times, most recently from 5b00a6a to 1e0393f Compare July 17, 2020 21:02
@NotWoods NotWoods closed this Jul 17, 2020
@NotWoods NotWoods reopened this Jul 17, 2020
Copy link
Contributor

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

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

🚀

@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@a13c6e8). Click here to learn what that means.
The diff coverage is 30.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #12675   +/-   ##
=========================================
  Coverage          ?   26.71%           
  Complexity        ?      940           
=========================================
  Files             ?      400           
  Lines             ?    15936           
  Branches          ?     2033           
=========================================
  Hits              ?     4257           
  Misses            ?    11353           
  Partials          ?      326           
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/org/mozilla/fenix/FenixApplication.kt 13.01% <0.00%> (ø) 4.00 <0.00> (?)
...pp/src/main/java/org/mozilla/fenix/HomeActivity.kt 6.86% <0.00%> (ø) 13.00 <0.00> (?)
...illa/fenix/addons/InstalledAddonDetailsFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...lla/fenix/components/toolbar/DefaultToolbarMenu.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...p/src/main/java/org/mozilla/fenix/home/HomeMenu.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...rc/main/java/org/mozilla/fenix/perf/Performance.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../quicksettings/QuickSettingsSheetDialogFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ix/trackingprotection/TrackingProtectionOverlay.kt 78.43% <0.00%> (ø) 10.00 <0.00> (?)
.../src/main/java/org/mozilla/fenix/utils/Settings.kt 61.25% <50.00%> (ø) 25.00 <1.00> (?)
...org/mozilla/fenix/components/BackgroundServices.kt 23.28% <75.00%> (ø) 0.00 <0.00> (?)
... and 4 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 a13c6e8...1188308. Read the comment docs.

@NotWoods NotWoods merged commit c08d375 into mozilla-mobile:master Jul 21, 2020
@NotWoods NotWoods deleted the settings-component branch July 21, 2020 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants