-
-
Notifications
You must be signed in to change notification settings - Fork 457
Description
In our CI, we're running reviewdog with the following command:
reviewdog -conf=.reviewdog.yml -reporter=github-pr-check -runners=Rubocop,Brakeman,Cspell -fail-on-error=true
And our .reviewdog.yml
looks like the following:
runner:
pronto-rubocop:
cmd: bundle exec pronto run --commit origin/${PR_TARGET_BRANCH:-develop} --runner rubocop
errorformat:
- "%A%f:%l %t: %m"
- "%Z```%$"
- "%C%.%#"
name: Rubocop
level: error
pronto-brakeman:
cmd: bundle exec pronto run --commit origin/${PR_TARGET_BRANCH:-develop} --runner brakeman
errorformat:
- "%f:%l %t: %m"
name: Brakeman
level: warning
cspell:
cmd: npx cspell lint ${PR_CHANGED_FILES} --config .cspell.yml --relative --no-summary --no-progress --no-must-find-files --gitignore || true
errorformat:
- "%f:%l:%c - %m"
name: Cspell
level: warning
With that setup, the behavior on v0.18.1:
- whenever we had an infraction on rubocop, we'd get an error exit code
- whenever we didn't have an infraction on rubocop, we'd get a success exit code (regardless of cspell/brakeman infractions)
With that setup, the behavior on >=v0.20.0:
Probably a related buggy behaviour:
Following the same setup, on >=v0.20.0
:
- when
-fail-on-error=true
: all the checks are reported to GitHub were reported with the default namereviewdog
instead of respecting the checkname
defined in the config file. And even more interesting, all the checks were failed cspell checks (there wasn't any rubocop or brakeman checks reported in my tests - where those checks should be green) - when
-fail-on-error=false
: only the cspell red check was reported to GitHub, and it was named with the default namereviewdog
instead of respecting the custom name defined in the config file.
In respect to the -fail-on-error
flag, I was a bit confused by this part of the README:
Exit codes
By default reviewdog will return 0 as exit code even if it finds errors. If -fail-on-error flag is passed, reviewdog exits with 1 when at least one error was found/reported. This can be helpful when you are using it as a step in your CI pipeline and want to mark the step failed if any error found by linter.
See also -level flag for github-pr-check/github-check reporters. reviewdog will exit with 1 if reported check status is failure as well if -fail-on-error=true.
My interpretation from that is, when using the github-pr-check
reporter alongside the -fail-on-error
flag, we should only get an error exit code if both of the following are satisfied:
- there was at least one infraction from any runner
- there was a failure check reported to GitHub API
So considering the interpretation above, I guess the new behavior is a bug (but would like to hear an input from @haya14busa to confirm)