clippy-check icon indicating copy to clipboard operation
clippy-check copied to clipboard

Add option to fail check on warnings

Open sffc opened this issue 4 years ago • 3 comments

Do the checklist before filing an issue:

  • [x] Is this related to the actions-rs Actions? If you think it's a problem related to Github Actions in general, use GitHub Community forum instead: https://github.community
  • [x] You've read the Contributing section about feature requests: https://github.com/actions-rs/.github/blob/master/CONTRIBUTING.md#feature-requests
  • [ ] Is this something you can debug and fix? Send a pull request! Bug fixes and documentation fixes are welcome.

Motivation

Currently, the check fails only when clippy returns an error. This makes it hard to notice when warnings occur, since Clippy warnings can also be helpful. As usual, noisy warnings can be disabled in code with #[allow(...)]. I would therefore like the option for the check to fail on both warnings and errors.

I would like this behavior both on in-repo PRs and forked PRs.

Workflow example

    - uses: actions-rs/clippy-check@v1
      with:
        token: ${{ secrets.GITHUB_TOKEN }}
        args: --all-targets --all-features
        failure-mode: warnings

Additional context

See discussion in https://github.com/unicode-org/icu4x/pull/161. Currently we are adding two different checks, the actions-rs version for the nice HTML in supported PRs, and a command line version with -- -D warnings to cause a failed check. It would simply and speed up our workflow if we didn't need the second check only for the check failure.

CC @echeran @Manishearth

sffc avatar Jul 01 '20 20:07 sffc

Hi, @sffc! Is there any reason not to add -D warnings directly to the args input, as in args: --all-targets --all-features -- -D warnings? That should (as usual) make clippy exit if there were any warnings, and as a consequence, fail the check too.

Speaking about the clippy annotations, there is a "nextgen" clippy Action exists, which should solve some of the problems you mentioned in the original issue (to be more specific, it will work for all PRs). Unfortunately, the only way to make it work right now is to use unstable GitHub feature (I'm actually not sure in what state it is right now, as the tracking issue is in stale state since October and GH staff is not very helpful in providing any sort of ETA), but if you up to some "potentially broken" scripts in your CI, you can give it a shot.

svartalf avatar Jul 01 '20 20:07 svartalf

@svartalf Earlier testing iterations of the workflow did indeed include ... -- -D warnings in the args to the clippy Action, but it did not fail the job / workflow as expected (test ex 1, test ex 2). The next test was successful after dropping down from the clippy Action to running Cargo at the CLI.

@sffc 's pasted snippet was taken from the current iteration of the workflow, after the clippy step had been split up into 2 jobs -- one uses the clippy Action to get the awesome Github UI integrations, the other job runs cargo clippy at the CLI in strict mode to get the job / workflow failures.

echeran avatar Jul 06 '20 16:07 echeran

When it comes to the warnings being treated as errors, I just realized that this functionality fix was already made in v1.0.2. Sorry about that.

That leaves us with the annotations concern, and that is too bad that we can't tell how the progress is coming along. Since that is the only part remaining here, I think we might be able to close this issue as a duplicate of #2 .

echeran avatar Jul 08 '20 22:07 echeran