-
-
Notifications
You must be signed in to change notification settings - Fork 516
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
Resolving design modifiers based on the theme type #4978
Conversation
Well I should have ran the tests locally first! It seems like this change broke a few things. Marking the PR as draft. |
Suspect sometimes the theme is passed in as the type and sometimes as the instance, so it just needs an isinstance check. |
855e17d
to
2c9525c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4978 +/- ##
===========================================
+ Coverage 22.35% 84.44% +62.09%
===========================================
Files 299 299
Lines 44647 44680 +33
===========================================
+ Hits 9980 37730 +27750
+ Misses 34667 6950 -27717
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and thanks for catching this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and thanks for catching this!
dd137de
to
7645f26
Compare
Found this while trying out objgraph on a dummy app (to find a memory leak on a bigger app) and it proved useful as it detected that opening a new session of the same app generated some new
NativeDefaultTheme
objects that weren't cleaned up. This happened because_resolve_modifiers
- which is decorated with@functools.lru_cache
- was passed a new instance of the theme instead of its type, while I believe passing the type is enough (I haven't followed all the modifiers related code though).