CI: Add a job that runs clang-format on PR changes #99
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce a script to check the coding style, and a CI worflow which runs it only on the changes introduced by a PR. The goal of checking only the changed code is to avoid having to run a full reformatting of the tree: instead, changes that modified existing code will slowly make the tree drift towards the desired status without introducing noise in the repository history. There are is some trickery involved in getting this to work reliably: - The "clang-format-diff" helper script is installed at different locations depending on the distribution. The "check-style" script checks a few locations that cover popular GNU/Linux distributions (Ubuntu, Debian, Arch Linux, Fedora); it can be improved as we see fit in the future. - The "actions/checkout" step defaults to shallow clones of the single commit which triggered a PR, which is enough to build, but not to find the differences between it and the base commit. To avoid forcing a full fetch of the history, the job uses a second instance of the action to fetch only the base tree in a separate directory. - Even with both trees checked out, Git will refuse to calculate a diff because the commits are not linked to one another. But both trees are now checked out, so instead it is possible to use a plain "diff" and feed its output to "check-style --diff". As a bonus, the "check-style" script can be run by developers locally from the command line in order to verify their changes prior to filing merge requests, e.g: % git co -b my-feature % $EDITOR cog.c % git ci -m'Awesome improvements' % scripts/check-style master The output from the script is an unified diff, which can be piped back into "patch" or "git apply" to get the suggested changes applied to the tree: % scripts/check-style master | git apply -p0 - % git add -p && git commit --amend
aperezdc
requested review from
carlosgcampos,
clopez,
philn,
pmatos and
zdobersek
Contributor
Author
|
The suggestion of adding a CI build for this came from @donny-dont and I agree it is a very good idea. The |
Merged
philn
approved these changes
Mar 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a script to check the coding style, and a CI worflow which runs it only on the changes introduced by a PR. The goal of checking only the changed code is to avoid having to run a full reformatting of the tree: instead, changes that modified existing code will slowly make the tree drift towards the desired status without introducing noise in the repository history.
There are is some trickery involved in getting this to work reliably:
The
clang-format-diffhelper script is installed at different locations depending on the distribution. Thecheck-stylescript checks a few locations that cover popular GNU/Linux distributions (Ubuntu, Debian, Arch Linux, Fedora); it can be improved as we see fit in the future.The
actions/checkoutstep defaults to shallow clones of the single commit which triggered a PR, which is enough to build, but not to find the differences between it and the base commit. To avoid forcing a full fetch of the history, the job uses a second instance of the action to fetch only the base tree in a separate directory.Even with both trees checked out, Git will refuse to calculate a diff because the commits are not linked to one another. But both trees are now checked out, so instead it is possible to use a plain
diffand feed its output tocheck-style --diff.As a bonus, the
check-stylescript can be run by developers locally from the command line in order to verify their changes prior to filing merge requests, e.g:The output from the script is an unified diff, which can be piped back into
patchorgit applyto get the suggested changes applied to the tree: