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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: passed arguments should take precedence over values in config #2100
Conversation
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.
What's the specific breaking change this will expose ... if it's behavior that people would never want, and no tests caught it, then I could imagine us not taking this as a major change -- worst case perhaps we could put this behavior behind a flag, to help people bumping into issues, and then swap the default value in the next major.
@bcoe I'm not sure I understand you correctly. Does your comment states this change will not be included until next major release unless it is hidden behid a flag or the other way around - it is for such an uncommon (rare) case that the author should not worry about any breakage? |
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.
Thinking on this one a bit, a couple things jump out at me:
- Rather than implicitly deciding whether to concat values, based on whether a default value has been configured for a key, better that we figure out a way to track (during parsing) whether a default value has been used -- we've talked in the past about adding an
isDefaulted
method, this would be a great feature for users, and would also allow us to simplify this logic. - Providing config feels different than default values, and I could imagine 50% of people wishing that values would be combined, and the other 50% wishing for the opposite behavior. This makes me think that rather than taking this as a breaking change, we'd do better to introduce a parser flag, to allow folks to configure which approach is taken.
Questions:
|
@jly36963 thank you for this great work 馃憦, I will make an effort to review soon. |
Relates to:
issue: #2096
PR: #1992
PR: #2006
Problem
There were a few problems with pull 1992:
Questions
BEGIN_COMMIT_OVERRIDE
fix: veriadic arguments override array provided in config (the same as multiple dash arguments).
END_COMMIT_OVERRIDE