Hello friends,
once upon a time:
> (21:35:09) CxByte: BenW: since you're the travis guy around here, how feasible is it to make it run clang-format and fail if anything changes?
> (21:45:04) BenW: CxByte: Precisely as difficult as getting clang-format available at all.
> (21:45:43) CxByte: Ah, basically impossible (until we bump up the image distro version?)
> (21:47:01) BenW: We probably should. Ubuntu Focal has been around for long enough, and has been working nicely for other projects all the time.
> (21:47:28) kling: not a bad idea :)
> (21:47:58) CxByte: well, consider that a feature request then :P
Upstream Serenity already uses Ubuntu Focal since #3483 [1]. Getting
clang-format to run is as simple as inserting the right apt-get install
command.
However, clang-format has a few false-positives.
Most false positives seem to revolve around assembly, and some weird
interactions between annotations like [[nodiscard]] and templates.
Inserting Something like "// @clang-format:off" (or whatever the syntax
is) seems reasonable.
*Is this the way to go, or is there a better approach?*
clang-format also finds *lots and lots* of true positives.
Currently I imagine fixing that as a train of commits like
- Meta+AK: Make clang-format-10 clean
- Meta+Applications: Make clang-format-10 clean
- Meta+Demos: Make clang-format-10 clean
and so on. It's going to screw up "git blame" in all cases, that's
unavoidable.
*Does someone have a better idea than this?*
Unless, of course, you tell me that we don't want a large "reformat", in
which case the entire thing becomes moot. :)
Cheers,
Ben
[1] https://github.com/SerenityOS/serenity/pull/3483
> false positives
We *could* also skip the files with false positives, or with a lot more
effort, test for such chunks (as you said, inline asm; nodiscard, etc.)
before failing the build.
> It's going to screw up "git blame" in all cases
There's a config option `blame.ignoreRevsFile`, which can be set to a
file containing a list of revs to ignore when `blame`-ing:
https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt
We could provide such a file, although I don't know if there's a way to
make github respect that file.
We could also leave existing mis-formats and only run git-clang-format
against PRs. This would solve the blame issue, and allow people to pass
CI even when master is malformatted.
This could be accomplished by the following script (I think)
if [[ ! -z $TRAVIS_PULL_REQUEST_SHA ]]; then
git-clang-format --diff $TRAVIS_PULL_REQUEST_SHA $TRAVIS_BRANCH
exit $?
fi