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
Structure and Ambiguous rule bundles #4444
Conversation
Pull Request Test Coverage Report for Build 4302599190
💛 - Coveralls |
Pull Request Test Coverage Report for Build 4304409563
💛 - Coveralls |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4444 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 208 210 +2
Lines 15420 15473 +53
=========================================
+ Hits 15420 15473 +53
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This query violates L027, L044, L050, L051, and L052. | ||
When we exclude L05*,L027 in the config we expect L027, L050, L051, | ||
and L052 to be ignored by the linter. | ||
This query violates L027, AM04 (ex L044), L050, AM05 (ex L051), and L052. |
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.
Not entirely sure we want zombie refs around where we don't need them?
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.
Ah - I mention them, because the globs in this test (L04*
), do still match these rules because of their aliases. It's not as much that they're there as old refs, it's because those refs persist as aliases which can be matched by globs (as demonstrated in this test).
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.
@WittierDinosaur - do you reckon that makes sense? I think otherwise the test is kind of confusing to understand.
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.
Okay, I guess so. But you don't do that in the glob_include tests
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.
Hmmmm good point. I'll update the comments to be more explicit on both.
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.
LGTM
Another step in #4031. These ones are all straightforward recodes. All the rules are still intact and just moved into new bundles as per the rules spreadsheet.
This also updates rule glob matching expressions in
rules
andexclude_rules
to also match on aliases, names and groups - mostly because it supports better backward compatibility and it's not significantly more complicated to execute.