-
Notifications
You must be signed in to change notification settings - Fork 13.1k
refactor: Remove settings dependency on startup #34199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #34199 +/- ##
============================================
+ Coverage 59.21% 75.13% +15.92%
============================================
Files 2824 519 -2305
Lines 67988 22944 -45044
Branches 15122 5537 -9585
============================================
- Hits 40257 17240 -23017
+ Misses 24902 5038 -19864
+ Partials 2829 666 -2163
Flags with carried forward coverage won't be shown. Click here to find out more. |
621797f to
63fef6e
Compare
There's a lint issue i didn't notice first :)
63d779b
Proposed changes (including videos or screenshots)
The idea is to remove code dependencies to "settings cached" during the startup that are spread everywhere in the codebase. This is basically but not only represented by code using
settingsin the file's top level scope.The currently method to make sure the settings cache is built before code is run is importing the "cached settings" from the file that builds the cache, so the top level await will ensure the cache is built before everything else runs. The proposal here is that we don't rely on this mechanism anymore by having a more clear code path, so we first build the cache and after that we import everything else.
So this is the first step of many to come, the changes are basically moving code relying on
settingsto functions and then calling this functions on the appropriate moment.Issue(s)
ARCH-1398
Steps to test or reproduce
Further comments