-
Notifications
You must be signed in to change notification settings - Fork 278
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
feat: add the ability to ignored keys while sorting #689
base: master
Are you sure you want to change the base?
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.
Hello J.M., thanks for contributing!
The idea looks good. I propose these modifications:
- Name the new option
ignored-keys
for explicitness (if we add other "ignored" options in the future). - Accept regexes instead of fixed strings, for more powerful usage. See for example the rule
quoted-strings
with its optionsextra-required
andextra-allowed
.
Also, can you include more tests? Especially for more than 1 ignored-keys
, and with the regex feature we should also check it, e.g. name
, aaa name
and name zzz
for ignored-keys: ['name']
vs. ignored-keys: ['^name$']
.
Finally, you need to add a .. rubric:: Options
in documentation (see other rules for examples).
@@ -88,7 +98,8 @@ | |||
|
|||
ID = 'key-ordering' | |||
TYPE = 'token' | |||
|
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.
Don't delete this line
I'm very motivated to get this merged and I see @fernandezcuesta hasn't responded in over a month. Would any be opposed to me making the reviewer comment changes in a new PR? /cc @adrienverge FYI my use case is GitHub Actions. Sorting some of the keys there makes things more confusing/harder to maintain for humans (read: bad). |
I'll do it, apologies for the long delay |
1a49252
to
b2f15a2
Compare
f7a0867
to
ffafaed
Compare
Hi, the use case here is to allow nested maps such as:
not to be sorted, since there are times when we consider a primary/merge-key useful to remain wherever we define it (usually in 1st place)