Skip to content

ci: Integrate github inline annotation for clippy #655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ci: Integrate github inline annotation for clippy #655

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16b7c2b
ci: integrate github inline annotation for clippy
jerensl Aug 15, 2025
edee035
ci: remove -D warnings flags
jerensl Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ jobs:
run: cargo fmt --all -- --check

- name: Run clippy
run: cargo clippy --all-features --all-targets -- -D warnings
uses: servo/actions/cargo-annotation@main
with:
cargo-command: clippy --all-features --all-targets
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

@jerensl jerensl Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add new PR here so we can add -- -D warnings when with-annotation is true, so our CI dont get cancel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


build_result:
name: Result
Expand Down