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

Improve streams for Gulp 4 #108

Open
brendanfalkowski opened this issue Mar 26, 2019 · 4 comments
Open

Improve streams for Gulp 4 #108

brendanfalkowski opened this issue Mar 26, 2019 · 4 comments

Comments

@brendanfalkowski
Copy link

brendanfalkowski commented Mar 26, 2019

I think there's an opportunity to make gulp-stylelint more Gulp-like by splitting linting and reporting into separate streams.

I recently migrated my "frontend starter" repo from Gulp 3 to 4, and volunteered as the guinea pig for Gulp Office Hours where @phated helped diagnose some issues with error handling and notifications (for better dev experience).

I still have an issue with gulp-stylelint — this manifests as the linter always ending successfully, so the notifier always fires a success (in addition to any warning/error). There's a good discussion going on here, and it's important to me because Stylelint is the best maintained linter for CSS:
gravitydepartment/frontend-starter#4

@olegskl — Do you have any ideas for this? Or would you be interested in participating in another Gulp Office Hours session to refactor?

@nirazul
Copy link

nirazul commented Mar 27, 2019

I think I might run in the same issue here. I'm trying to make the reporter fail on warnings, in order to make the CI build fail. But unfortunately my custom reporter isn't able to make the pipe fail.

@olegskl
Copy link
Owner

olegskl commented Apr 2, 2019

@brendanfalkowski The plugin emits a PluginError which causes the stream to fail with non-zero code on errors by default. You can test it on https://github.com/olegskl/gulp-stylelint-test. I don't know how splitting the linting and reporting into separate streams may resolve the issue, but I'm willing to discuss it if does not involve the condescending tone I noticed in the linked issue.

@nirazul Perhaps you can use a different configuration for your CI that elevates severity of warnings?

@brendanfalkowski
Copy link
Author

brendanfalkowski commented Apr 4, 2019

@olegskl — I'm not sure exactly how splitting the streams helps, but I'm just passing on the feedback from @phated to find a solution and document getting a nicer local dev experience for others to reference. Consider me just a loving user of these tools, but I don't have the same depth with JS streams as the creators.

Something is handled differently because in my Gulp 3 build gulp-stylelint would not reach the success notification if linter errors were encountered. Example events:

  1. Linter starts checking CSS
  2. Errors were found
  3. Errors are logged to the console
  4. Notification fires (to MacOS) of the error

In Gulp 4, the errors are reported but a success notifier also fires (right after the failing notifier).

  1. Linter starts checking CSS
  2. Errors were found
  3. Errors are logged to the console
  4. Notification fires (to MacOS) of the error
  5. Notification fires (to MacOS) of the success

I'm guessing this happens because the "lintCss" task is finishing, and that's what it's supposed to do when the linter runs and passes:

  1. Linter starts checking CSS
  2. All good
  3. Notification fires (to MacOS) of the success

This could very well be my Gulpfile's patterns or my error formatter. Both of them changed between Gulp 3 and 4, but my new build is using fewer packages and more native Gulp/Node functions.

I was hoping it was a simple config change, but the options I tried didn't help:
gravitydepartment/frontend-starter#4 (comment)

@nirazul
Copy link

nirazul commented Apr 5, 2019

@olegskl I would prefer to keep one single configuration as I do with gulp-eslint. I think the only thing that needs to change is to take into consideration the --max-warnings flag of stylelint.

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

No branches or pull requests

3 participants