Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

Add option to fail check on warnings #82

Open
2 of 3 tasks
sffc opened this issue Jul 1, 2020 · 3 comments
Open
2 of 3 tasks

Add option to fail check on warnings #82

sffc opened this issue Jul 1, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@sffc
Copy link

sffc commented Jul 1, 2020

Do the checklist before filing an issue:

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 unicode-org/icu4x#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

@svartalf
Copy link
Member

svartalf commented Jul 1, 2020

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.

@echeran
Copy link

echeran commented Jul 6, 2020

@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
Copy link

echeran commented Jul 8, 2020

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 .

mjkoo added a commit to mjkoo/cbors that referenced this issue Jul 22, 2021
mjkoo added a commit to mjkoo/cbors that referenced this issue Jul 22, 2021
* Remove travis config

* Add actions

* Try maturin from tox

* Disable coverage for now

* Deny clippy warnings

* Fix clippy command, fix audit

* Add two clippy steps to work around actions-rs/clippy-check#82

* Fix clippy warnings

* Fix fmt
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Development

No branches or pull requests

3 participants