sasstools / gulp-sass-lint

Gulp plugin for Sass Lint
MIT License
115 stars 43 forks source link

`max-warnings` is not reflected in gulp-sass-lint #73

Open chris-rock opened 7 years ago

chris-rock commented 7 years ago

The current release of gulp-sass-lint is not taking the option max-warning into account:

Assume the following .sass-lint.yml

options:
  max-warnings: 0

I run the test with sass-lint directly:

$sass-lint  -v -q -c .sass-lint.yml elements/**/*.scss

report/report.scss
  7:3  warning  Expected `display`, found `font-family`  property-sort-order

✖ 1 problem (0 errors, 1 warning)

$ echo $?                                                             
1

Now with gulp, using the same .sass-lint.yml:

$ gulp sasslint
[14:22:22] Using gulpfile ~/report/gulpfile.js
[14:22:22] Starting 'sasslint'...
[14:22:22] Finished 'sasslint' after 9.16 ms

report/report.scss
  7:3  warning  Expected `display`, found `font-family`  property-sort-order

✖ 1 problem (0 errors, 1 warning)

$ echo $?      
0

Please advice, how to proceed?

chris-rock commented 7 years ago

This is implemented in sass-lint https://github.com/sasstools/sass-lint/blob/240a148cba44abd7c784f72ae7839cdb9cc3be60/bin/sass-lint.js#L13-L17

chris-rock commented 7 years ago

@Snugug can you share you insights on this?

webdevian commented 7 years ago

It doesn't seem to follow 'max-warnings': 0 in the gulp task options either.

webdevian commented 7 years ago

I've tried to find some workarounds but this omission from the standard sass-lint behaviour makes the plugin pretty useless to me.

I'm better off using exec('sass-lint -v') because it actually follows my config as expected

chris-rock commented 7 years ago

@DanPurdy Is there any guidance regarding this topic? I hesitate to open a new PR, just to get this closed again

DanPurdy commented 7 years ago

Off the top of my head (i've not had the time to even look at sass-lint recently) it's because the gulp implementation is doing this on a file by file basis. If you were to reach the max errors in a single file you may see it work and then carry on linting the next file. Which is why the method @webdevian is showing is working as it'll handle it through the CLI.

I would guess that we'd need to incorporate some kind of collation of the results and then handle this separately. Trying to do that tidily without hitting the limit as I described above though might be a problem.

webdevian commented 7 years ago

This is what I ended up using. I have my .sass-lint.yml max-warnings set to 0. I haven't tested it with setting that to any other value.

gulp.task('sass-lint', function(cb) {
  exec('sass-lint -v', function(err, stdout) {
    gutil.log(stdout);
    cb(shouldItExitVar ? err : 0);
  });
});
Gidmark commented 7 years ago

Either make this work or bring back the failOnWarning imho

gconry18 commented 4 years ago

Is this project now abandonware? Would be nice to get the ability to failOnWarning for our CI process.