-
Notifications
You must be signed in to change notification settings - Fork 235
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
Update --print-pending
logic to ignore existing pending modules that have no linting errors
#954
Update --print-pending
logic to ignore existing pending modules that have no linting errors
#954
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.
Thanks for working to fix this!
bin/ember-template-lint.js
Outdated
@@ -166,7 +166,13 @@ function run() { | |||
|
|||
if (printPending) { | |||
let failingRules = Array.from( | |||
fileErrors.reduce((memo, error) => memo.add(error.rule), new Set()) | |||
fileErrors.reduce((memo, error) => { | |||
if (error.rule) { |
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.
Instead of making this kindof implicit, can we add a special .code
/ .rule
value that we can check for?
I'm thinking we can tweak this:
ember-template-lint/lib/index.js
Lines 174 to 178 in 549775f
messages.push({ | |
message: `Pending module (\`${options.moduleId}\`) passes all rules. Please remove \`${options.moduleId}\` from pending list.`, | |
moduleId: options.moduleId, | |
severity: 2, | |
}); |
And add a rule: 'invalid-pending-module'
or something.
What do you think?
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.
Great suggestion! Should be more clear now.
Friendly ping @gmurphey 😸 |
288690e
to
77123ff
Compare
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.
Awesome, thank you!
--print-pending
logic to ignore existing pending modules that have no linting errors
Was changing my rule configuration and running
ember-template-lint . --print-pending
to generate an updated pending list of files and noticed that there were some entries that looked like this:Debugging led me to realize these were being added to the output for
--print-pending
due to an error being thrown during linting sayingPending module ('my/path/to/a/template') passes all rules. Please remove 'my/path/to/a/template' from pending list.
This PR adds a check when building the output for
--print-pending
to make sure that the error thrown by the linter has a rule.Thanks for this great tool!