Stream: git-cranelift

Topic: cranelift / Issue #1170 Integrate clippy checking in CI b...


view this post on Zulip GitHub (Jan 10 2020 at 21:51):

abrown commented on Issue #1170:

Just looked at this a bit more and I think someone with rights should install https://github.com/marketplace/actions/rust-clippy-check. That GitHub action will use clippy to check PRs and post any issues as annotations (follow the previous link for an example). I think to use this we need to use a GitHub secret token, see https://github.com/marketplace/actions/rust-clippy-check#example-workflow.

view this post on Zulip GitHub (Jan 13 2020 at 10:07):

bnjbvr commented on Issue #1170:

Interesting. I'm only wondering what this will do to existing Clippy warnings: will they be reported as part of a PR that modifies code around, or will only new Clippy warnings be reported? In the former case, I think it would be nicer to first fix all the Clippy warnings before enabling such a check.

view this post on Zulip GitHub (Jan 13 2020 at 17:35):

abrown commented on Issue #1170:

I suspect we would have to fix all the clippy warnings first but that wasn't too difficult: #1340.

view this post on Zulip GitHub (Feb 26 2020 at 19:59):

abrown commented on Issue #1170:

@alexcrichton, what do you think about adding this to cranelift and wasmtime, perhaps when they merge?

view this post on Zulip GitHub (Feb 26 2020 at 22:27):

alexcrichton commented on Issue #1170:

I personally agree with @fitzgen's thoughts above where hard-gating on clippy on CI is often quite onerous. I personally disagree with a lot of clippy's lints as well which doesn't help my own opinion though... I would advocate for a "here's how to run it if you want" at most, but otherwise don't hard-gate it on CI.

view this post on Zulip GitHub (Feb 27 2020 at 03:48):

abrown commented on Issue #1170:

:+1: I agree with the "no hard-gating" point of view as well. I think the point of installing this clippy action would be for awareness more than anything else: instead of running clippy in CI, ignoring failures, and thus doing extra work that is ignored, this action would make PR submitters and reviewers aware of what clippy thinks is problematic. If clippy finds a real issue--good; if clippy finds irrelevant stuff then they can ignore the warning (locally or globally).

view this post on Zulip GitHub (Feb 27 2020 at 04:08):

philipc commented on Issue #1170:

The limitations on https://github.com/marketplace/actions/rust-clippy-check make it seem to be not worth the effort: "Due to token permissions, this Action WILL NOT be able to post clippy annotations for Pull Requests from the forked repositories."

view this post on Zulip GitHub (Feb 27 2020 at 17:43):

abrown commented on Issue #1170:

Hm, yeah, then there's not much point to doing this until that is fixed; seems like this can issue can go forward when/if https://github.com/actions-rs/clippy-check/issues/2 is fixed.

view this post on Zulip GitHub (Feb 28 2020 at 23:27):

alexcrichton transferred Issue #1170 (assigned to abrown):

Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue!


Last updated: Dec 23 2024 at 13:07 UTC