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 . 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  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