search

JustBlackBird / gulp-phpcs

Gulp plugin for running PHP Code Sniffer
MIT License
47 stars 12 forks source link

fix: fail reporter #28

Closed di5abled closed 7 years ago

di5abled commented 7 years ago

only fail at the end of the pipe that other reporters have the possibility to make their output and don't get canceled by the fail reporter

di5abled commented 7 years ago

fix for https://github.com/JustBlackBird/gulp-phpcs/issues/27

JustBlackBird commented 7 years ago

In general it LGTM. But I have some thoughts about the PR:

  1. Have you considered implementing "fail on last behavior" as a flag (ex. failOnFirst) for fail reporter constructor? In such case we can keep the current behavior (as default failOnFirst: true) of the reporter but add feature you need.
  2. To make sure your changes do what they should you should add new tests for new functionality.
  3. Your PR broke the build, so I just cannot merge it until the build is broken.
JustBlackBird commented 7 years ago

Thank you for the PR.