-
Notifications
You must be signed in to change notification settings - Fork 53
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
CLI: Add --exclude-checks
support
#318
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.
@swissspidy Almost LGTM, only I think we should choose a consistent name for the new argument.
@@ -62,6 +62,10 @@ public function __construct( Plugin_Context $plugin_context ) { | |||
* [--checks=<checks>] | |||
* : Only runs checks provided as an argument in comma-separated values, e.g. i18n_usage, late_escaping. Otherwise runs all checks. | |||
* | |||
* [--exclude-checks=<checks>] |
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.
You call this ignore-checks
/ ignore-slugs
everywhere else in this PR. I don't feel strongly about either way, but we should go with a consistent term throughout. It makes sense that the parameter uses "checks" while the code uses "slugs", but I mean we should only use the term "ignore" or "exclude" throughout.
I personally prefer exclude-checks
I think, but either way WFM.
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.
Makes sense, let's settle on exclude
—exclude-checks
support--exclude-checks
support
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.
Thanks @swissspidy, LGTM!
@adamsilverstein Just as an FYI, I'll update the GitHub Action right after merging this one. |
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.
Fantastic!
Fixes #308
Successfully tested using something like this: