fail_on_error works with lint warnings

[sa9sha9]

I guess it should work only with lint errors. Is it possible to fail only on error?

My reviewdog.yml is below.

name: reviewdog
on: [pull_request]
jobs:
  eslint:
    name: runner / eslint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: eslint
        uses: reviewdog/action-eslint@v1
        with:
          reporter: github-pr-review
          workdir: "app/"
          eslint_flags: "--cache --ext .js,.jsx,.ts,.tsx ."
          level: error
          fail_on_error: true

[narrowizard]

I think it's eslint stuff. add --quiet cli option for eslint, and it works well.
https://eslint.org/docs/user-guide/command-line-interface @sa9sha9

name: reviewdog
on: [pull_request]
jobs:
  eslint:
    name: runner / eslint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: eslint
        uses: reviewdog/action-eslint@v1
        with:
          reporter: github-pr-review
          workdir: "app/"
          eslint_flags: "--quiet --cache --ext .js,.jsx,.ts,.tsx ."
          level: error
          fail_on_error: true

[hylle]

@narrowizard With --quiet warnings aren't reported at all.
I think what @sa9sha9 is looking for is way to have error level findings cause a fail, while still having a warnings reported, but without having it cause a fail.

[carmanchris31]

@narrowizard With --quiet warnings aren't reported at all. I think what @sa9sha9 is looking for is way to have error level findings cause a fail, while still having a warnings reported, but without having it cause a fail.

This is the main issue I have with the action also. Eslint only fails the job when errors are reported and it's reasonable to expect (or at least allow) the same behavior from this action.

[carmanchris31]

Looks like this may be a limitation of Github and they have adding support for it in their backlog:
community/community#9875 (comment)

[carmanchris31]

Actually It looks like this is already supported:

The final conclusion of the check. Can be one of action_required, cancelled, failure, neutral, success, skipped, stale, or timed_out
—source

Here's an example of this being used in the wild:
https://github.com/wearerequired/lint-action/blob/2fe6593ac19ccad08133cf11685d5051fa94bbba/src/github/api.js#L44-L53

And reviewdog is also using it, but unfortunately it looks like it only checks the total count of annotations when determining the conclusion of the run:
https://github.com/reviewdog/reviewdog/blob/ff5f2741c6ddd67889710ab061ce323de776f2fa/doghouse/server/doghouse.go#L101-L103
https://github.com/reviewdog/reviewdog/blob/ff5f2741c6ddd67889710ab061ce323de776f2fa/doghouse/server/doghouse.go#L162-L169

It should determine the conclusion based on the highest severity reported.

[zachnicoll]

Would LOVE this to be merged so it behaves as ESLint would behave locally!

[fmatzy]

It appears that this issue has been resolved as of reviewdog v0.14.2.
However, since the default value for the level parameter in this action is still set to error, it's necessary to explicitly set the level value to an empty string when utilizing this action.

[javierjulio]

Perhaps worth considering changing the default for a new major version?

[houserx-jmcc]

Perhaps worth considering changing the default for a new major version?

Yeah, this feels like the right path as a default