-
-
Notifications
You must be signed in to change notification settings - Fork 313
fix: try to use ChainMap to chain settings, fix message_length_limit inconsistency
#1812
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
message_length_limit inconsistency (追記ここまで)
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.96%. Comparing base (de24815) to head (96caf65).
✅ All tests successful. No failed tests found.
Additional details and impacted files
@@ Coverage Diff @@ ## master #1812 +/- ## ======================================= Coverage 97.96% 97.96% ======================================= Files 60 61 +1 Lines 2648 2657 +9 ======================================= + Hits 2594 2603 +9 Misses 54 54
☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.
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.
Add some docstrings here:
- What is this
- What it does
- How to use
- How it compares to the previous solution
That would really help me understand the benefit.
Something I'd like to know as well:
Right now, if I make a plugin, I can create custom settings, but they are not typed. Would this allow plugin to declare their own settings?
example
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.
Right now, if I make a plugin, I can create custom settings, but they are not typed. Would this allow plugin to declare their own settings?
Thanks, I will put those scenarios into consideration. ChainMap approach should not break the type system.
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.
How it compares to the previous solution
We can discuss this in #1672. I will share the motivation there.
Uh oh!
There was an error while loading. Please reload this page.
Relate #1672