Skip to content
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.

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

.github: Update the style checker action for CheriBSD #2067

Merged
merged 6 commits into from
Apr 9, 2024

Conversation

bsdjhb
Copy link
Collaborator

@bsdjhb bsdjhb commented Mar 20, 2024

This is very expensive to run against merges from dev to main. Even
for PRs against dev we would only want it to be advisory (and wouldn't
want it for upstream FreeBSD merge PRs).

@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 20, 2024

Actual testing of this is over in a child PR against this branch here: bsdjhb#1

@bsdjhb bsdjhb marked this pull request as draft March 25, 2024 21:43
@bsdjhb bsdjhb force-pushed the disable_style branch 2 times, most recently from 29491a2 to c3ae5df Compare March 25, 2024 21:53
bsdimp added 5 commits March 25, 2024 15:23
If you add "::error file=foo/bar.c:line=123:" before the error message,
it will appear inline.

Sponsored by:		Netflix

(cherry picked from commit 0949b23)
Change the : between file and line to a ,. This should fix this...

Sponsored by:		Netflix

(cherry picked from commit c2f5306)
Remove extra space...

Sponsored by:		Netflix

(cherry picked from commit ac9abe9)
Use ::error and ::warning as appropriate to give better meaning to the
messages.

Sponsored by:		Netflix

(cherry picked from commit 22d12ca)
Let's see if we can get the style issues flagged inline.

Sponsored by:		Netflix

(cherry picked from commit 33326dd)
@bsdjhb bsdjhb marked this pull request as ready for review March 25, 2024 22:24
@bsdjhb bsdjhb requested a review from jrtc27 March 25, 2024 22:24
@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 25, 2024

This cherry picks several fixes back from FreeBSD to add --github support to get per-line annotations and then updates the yaml file for CheriBSD to run against dev instead of main, and to also always be green by forcing a return value of true.

You can see a sample of the per-file annotations it adds over at https://github.com/bsdjhb/cheribsd/pull/1/files

Note that it runs the checks on each commit, so if you "fix" a style bug in a later commit, you still end up with an annotation at the original location for the bug in the first commit.

@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 25, 2024

I looked btw and the clang-format action we use in the disabled clang-format check doesn't support GitHub annotations (I would have tried to fix it to do so otherwise). I think we should probably just remove the clang-format check if we adopt the changes in the PR.

@bsdjhb bsdjhb changed the title .github: Remove the style checker action .github: Update the style checker action for CheriBSD Mar 25, 2024
- Run for PRs against dev instead of main.

- Make the check always succeed but still add annotations.

- Check the style of the total diff instead of each patch on the branch.
@bsdjhb
Copy link
Collaborator Author

bsdjhb commented Mar 27, 2024

Now updated to only run the style check against the final diff so if a bug gets "fixed" in the middle it doesn't get annotated.

@bsdjhb bsdjhb merged commit de675c0 into CTSRD-CHERI:dev Apr 9, 2024
9 checks passed
@bsdjhb bsdjhb deleted the disable_style branch April 9, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants