Skip to content

fix: try to use ChainMap to chain settings, fix message_length_limit inconsistency - #1812

Draft
bearomorphism wants to merge 1 commit into
masterfrom
chain-settings-0119
Draft

fix: try to use ChainMap to chain settings, fix message_length_limit inconsistency#1812
bearomorphism wants to merge 1 commit into
masterfrom
chain-settings-0119

Conversation

@bearomorphism

@bearomorphismbearomorphism commented Jan 19, 2026

Copy link
Copy Markdown
Collaborator

Relate #1672

@bearomorphismbearomorphism changed the title refactor: try to use ChainMap to chain settingsfix: try to use ChainMap to chain settings, fix message_length_limit inconsistencyJan 19, 2026
@codecov

codecovBot commented Jan 19, 2026

Copy link
Copy Markdown

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.

from commitizen.defaults import DEFAULT_SETTINGS, Settings


class ChainSettings:

@woilewoileJan 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add some docstrings here:

  1. What is this
  2. What it does
  3. How to use
  4. 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

Copy link
Copy Markdown
CollaboratorAuthor

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.

Copy link
Copy Markdown
CollaboratorAuthor

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.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@bearomorphism@woile