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

Add option 'warningsAreErrors' for failing on both errors AND warnings. #31

Closed
wants to merge 1 commit into from

Conversation

DirtyHairy
Copy link

Hi all!

This PR adds an option warningsAreErrors for treating linter warnings as fatal errors.

Cheers
-Christian

@DanPurdy
Copy link
Member

Can I ask why? Can't you just set your rules to errors instead of warnings? I'm not sure plugin specific options are the way to go either. Happy to discuss though.

@DirtyHairy
Copy link
Author

Hi @DanPurdy !

I agree, you could just bump the severity of all rules to 2 and be done with. However, for me, defining the severity of a violation is complementary to configuring the maximum severity tolerated by the tooling. From a more practical side, this allows to use a single definition file in multiple contexts, i.e. tolerate warnings in development builds while disallowing them in production. To me, this is similar to specifying --max-warnings on the sass-lint command line.

Of course, from this viewpoint, warningsAreErrors is a misnomer, warningsAreFatal would be better (I was going to change this when I noticed your reply). An option maxWarnings similar to the sass-lint options would also be an alternative.

What do you think?

@DanPurdy
Copy link
Member

DanPurdy commented Oct 21, 2016

So with the next release of Sass-lint (1.10) you'll be able to use max-warnings in your config / passed in options too so that may achieve what you're looking for if I understand it. Currently yes it's only a CLI option but it will be available to you shortly hopefully.

So you could have your config set as you wish and then just pass in this extra parameter with your build task without requiring a separate grunt specific option. Would that solve this?

If you'd like to try it now you could just point your grunt sass lint package file at our develop branch instead of a NPM release.

@DirtyHairy
Copy link
Author

Hi @DanPurdy !

Yes, turning max-warnings is turned into a proper parameter for sass-lint would also solve the problem. I'll give it a try as I find time and keep you updated 😏

Cheers
-Christian

@DanPurdy
Copy link
Member

It's already done and will be released in 1.10 which hopefully wont be more than a week or two away.

@wingy3181 wingy3181 mentioned this pull request Nov 29, 2016
@DanPurdy DanPurdy closed this Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants