Skip to content

Conversation

@SimenB
Copy link
Contributor

@SimenB SimenB commented Aug 17, 2015

As discussed in #21

A way to write the report to file would be great, especially for reporters like junit and checkstyle. I came over https://github.com/juanfran/gulp-scss-lint which writes ti file, but not too cleanly... Other ideas for the API?

On a related note, I'd like to pass options in #29, we could put it in there?
EDIT: If you look at the built in formatters (https://github.com/CSSLint/csslint/tree/master/src/formatters) they take an optionshash. Could just use that? I've already added it in the other PR, so for this it's just be the case of reusing that options-hash

/cc @joshuacc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from me trying to mock out console... Does nothing that I can tell, as we manually restore the stub

@joshuacc
Copy link

👍

@lazd
Copy link
Owner

lazd commented Aug 18, 2015

1. Second argument

As proposed in this pull request.

gulp.task('lint', function() {
  gulp.files('lib/*.css')
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', 'csslint-junit-report.xml'));

2. Separate reporters

To add this ability to gulp-jshint, folks started publishing versions of the reporters that write to files gulp-jshint-file-reporter.

gulp.src('./lib/*.js')
  .pipe(jshint())
  .pipe(jshint.reporter('gulp-jshint-file-reporter', {
    filename: __dirname + '/jshint-output.log'
  }));

3. Plugin option

In gulp-scss-lint, they're passing an option to the linter itself, which feels like Grunt, not gulp.

gulp.src(['**/*.scss'])
  .pipe(scsslint({
    'reporterOutput': 'scssReport.json'
  }));

4. Capture stdout

There is a gulp-log-capture, but it's not a very simple way to get the job done and will probably taint the reporter output by including the entire task log.

gulp.src('src/js/*.js')
  .pipe(jshint())
  .pipe(logCapture.start(console, 'log'))
  .pipe(jshint.reporter('jslint_xml'))
  .pipe(logCapture.stop('xml'))
  .pipe(gulp.dest('lint-reports'));

@SimenB
Copy link
Contributor Author

SimenB commented Aug 18, 2015

1: In #29 an options-hash will be added as a second argument (should have been added in #21...), so we could piggy-back on that
2: Then we'd have to either call the built-in formatter again, or allow calling the reporter as well (feasible, I suppose)
3: Yeah, kinda. But as noted, the built-in formatters takes an options-hash, we could use that ourselves
4: That'll probably kill everything parseable about the report, thus rendering the feature virtually useless

@lazd
Copy link
Owner

lazd commented Aug 18, 2015

@SimenB, check out how reporters work in gulp-escomplex -- they always create a File and send it down stream.

I talked with @phated, and another option is to always send a File down stream, and make printing configurable (on by default).

Using the options hash as a second argument, as you proposed, this would end up being something like:

gulp.task('lint', function() {
  gulp.files('lib/*.css')
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false })) // don't print to stdout
    .pipe(gulp.dest('results/'));

I suppose it's possible that we won't want to send the report downstream, but instead the files themselves... Is that a valid use case?

@SimenB
Copy link
Contributor Author

SimenB commented Aug 18, 2015

The messes up linting files on the way to a final destinal (love those movies...), though, unless involving some fancy filtering and stuff.
It'd work for my use-case atm, as I only use gulp for linting and testing, while webpack handles the whole "building and moving files" part, but I'm guessing there are a few people that lint the css-files on their way to some build or dist directory, and it would complicate their workflow.

How about if stdout === false we attach the formatted report to the file (instead of printing it), then provide a built-in "write-to-disk-reporter" (your option nr 2 earlier), and bundle that like the fail-reporter?

@phated
Copy link

phated commented Aug 18, 2015

Write to disk reporters are against gulp guidelines. You are not supposed to touch the file system outside src/dest. There are modules like gulp-filter and gulp-if for the scenarios you mentioned.

@SimenB
Copy link
Contributor Author

SimenB commented Aug 18, 2015

Would it look something like this? I can't wrap my head around how to use filter or if to by-step the destructive operation. Am I missing something obvious?

gulp.task('css', function () {
  var cssFiles = gulp.src('src/styles/*.css');

  cssFiles
    .pipe(clone()) // gulp-clone
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false }))
    .pipe(gulp.dest('junit-report.xml'));

  return cssFiles
    .pipe(/*keep on piping the css-files, doing concat or whatever*/);
});

EDIT: I suppose we could ADD the file to the stream, then use filter/if to pipe it through gulp.dest. Does that make more sense, perhaps?

gulp.task('css', function () {
  var filter = gulpFilter('!csslint-report.xml');

  return gulp.src('src/styles/*.css');
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false, fileName: 'csslint-report.xml' }))
    .pipe(filter())
    .pipe(gulp.dest('results/'))
    .pipe(filter.restore())
    .pipe(/*keep on piping the css-files, doing concat or whatever*/);
});

@adamkingit
Copy link

We have this need as well. An implementation similar to idea 1 that @lazd proposed on Aug 18 above would both allow the user to capture the output of any reporter, and allow more than one reporter to be chained like below:
.pipe(csslint())
.pipe(csslint.reporter())
.pipe(csslint.reporter('junit-xml', {filePath:"test/lintcss.xml"}))
.pipe(csslint.reporter('fail'))

How can I help with this request?

@SimenB
Copy link
Contributor Author

SimenB commented Oct 29, 2015

@adamkingit Nag on me to complete #36 which includes the possibility to write to file.

EDIT: Or that seems complete (been a while since I worked on this). @lazd, anything holding up #36?

@SimenB
Copy link
Contributor Author

SimenB commented Feb 28, 2016

Closing this, as it's covered in a cleaner way by #36

@SimenB SimenB closed this Feb 28, 2016
@SimenB SimenB deleted the write-report-to-file branch February 28, 2016 17:58
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.

5 participants