Skip to content
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(lint): add max-warnings for cli-plugin-eslint, close #1268 #1289

Merged
merged 4 commits into from
May 18, 2018

Conversation

jkzing
Copy link
Member

@jkzing jkzing commented May 14, 2018

add a --max-warnings and --max-errors option for cli-plugin-eslint to specify number of warnings or errors to trigger build failed.

usage:

vue-cli-service lint --max-warnings 10 --max-errors 2

#1268

https://eslint.org/docs/user-guide/command-line-interface#options

@BenoitAverty
Copy link

The action on the status code is missing I think. The goal is to make the CLI exit with status >0 when the number of warings is greater than the parameter.

@jkzing
Copy link
Member Author

jkzing commented May 16, 2018

@BenoitAverty

The action on the status code is missing I think

Sorry but I didn't get your point...what is your expected behavior...😂

make the CLI exit with status >0 when the number of warings is greater than the parameter

Yes, it is now.

@@ -19,7 +20,7 @@ module.exports = function lint (args = {}, api) {
CLIEngine.outputFixes(report)
}

if (!report.errorCount) {
if (report.errorCount <= (argsConfig.maxWarnings || 0)) {
Copy link

@BenoitAverty BenoitAverty May 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does report.errorCount includes warnings ?

@BenoitAverty
Copy link

The expected behaviour is :

  • When there is one or more error, exit with status 1 (this is already the case)
  • When the number of warnings is greater than the parameter, exit with status 1 (This isn't the case currently, warnings don't make the exit code change to 1)
  • Otherwise, exit with status code 0

Examples:

# With 0 errors and 1 warning
vue-cli-service lint --max-warnings=1 # success.
vue-cli-service lint --max-warnings=0 # failure <-- this doesn't work currently

# With 1 error
vue-cli-service lint --max-warnings=0 # failure
vue-cli-service lint --max-warnings=1 # still failure (max warnings is only about warnings)

@jkzing
Copy link
Member Author

jkzing commented May 16, 2018

@BenoitAverty
I think I did misunderstand the usage of --max-warnings in eslint cli.

In eslint cli, --max-warnings only take effect on warnings, for errors the cli always exit with code 1.

But in current cli-plugin-eslint, it only exit 1 for errors, and for result that only contain warnings, it always exit 0.

So I think count both warnings and errors in --max-warnings is a little bit confusing, --max-errors would be more suitable here.

@BenoitAverty
Copy link

This should do the trick :) Thanks for this.

@yyx990803 yyx990803 merged commit ab877a2 into vuejs:dev May 18, 2018
@jkzing jkzing deleted the max-warnings branch May 21, 2018 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants