ci: Integrate github inline annotation for clippy #655
Conversation
Signed-off-by: Jerens Lensun <jerensslensun@gmail.com>
Signed-off-by: Jerens Lensun <jerensslensun@gmail.com>
| run: cargo clippy --all-features --all-targets -- -D warnings | ||
| uses: servo/actions/cargo-annotation@main | ||
| with: | ||
| cargo-command: clippy --all-features --all-targets |
There was a problem hiding this comment.
Is it correct that action will only fail the build as long as annotations are enabled? If annotations are ever disabled using with-annotations: false then only cargo clippy will be run but without the -D warnings that this CI setup is using. Is there a way we can fail the build even if annotations are disabled?
There was a problem hiding this comment.
We can still use -D warnings when with-annotations is set to false. This flag only causes issues if --message-format=json is specified after -- -D warnings. It only works when placed before it.
There was a problem hiding this comment.
Not sure I follow. The issue I'm referring to is the behaviour of cargo clippy when -D warnings is not passed (as in our action). Without -D warnings, cargo clippy will exit successfully even when there are lint violations and won't block the build.
In the current setup in this repo, -D warnings is being passed explicitly to make cargo clippy fail when there are violations. But in our action, we don't do that and rely on the annotations.sh to exit 1 if there are violations. However, that doesn't happen if annotations get disabled.
The question is if we can change the action to always fail, even if with-annotations is false.
There was a problem hiding this comment.
I add new PR here so we can add -- -D warnings when with-annotation is true, so our CI dont get cancel
There was a problem hiding this comment.
I'm not sure that we need to support explicitly passing -- D warnings in the action in order to preserve the behavior here. I'm thinking maybe we just execute annotations.sh always in the action, regardless of with-annotations being true or false, but only echo the GH annotation logs if with-annotations is true.
@jerensl have you tested this change with the recent changes to servo/actions/cargo-annotations as well? |
I haven't tested on this repo as the PR just merged yet, but im testing that on other repo here before it getting merged |
|
I guess we can trigger another test run from a fork of this repo now that more changes have been merged to servo/actions. |
Recently, we decided to centralize reusable actions in
github/actionsto make maintenance and updates easier across repositories. As part of this change, we are now adding Clippy annotations to some repos to make it easier to view errors directly in PRs.servo/ipc-channel#405 (comment)
This should produce an error whenever a warning is present, regardless of the
-D warningsflag.Testing PR: jerensl#2
Tested CI: https://github.com/jerensl/html5ever/actions/runs/17286983289/job/49065906417?pr=2#step:6:1