Skip to content

Consistent developer tooling; refactor formatting CI#3315

Merged
ZedThree merged 10 commits intonextfrom
pre-commit-hooks
Mar 12, 2026
Merged

Consistent developer tooling; refactor formatting CI#3315
ZedThree merged 10 commits intonextfrom
pre-commit-hooks

Conversation

@ZedThree
Copy link
Member

TL;DR:

CI will now fail on unformatted commits. Instead, run automatic formatting when you make commits:

uv sync --only-dev         # Install consistent versions of clang-format, etc
uv tool run prek install   # Install pre-commit hooks

clang-format and clang-tidy are both essential tools for keeping the code clean and catching potential bugs, but the way they are currently run in CI is getting a bit annoying. Namely:

  1. The last CI check ends up being the automated commit applying formatting, which always passes, and hides the actual test results
  2. The formatting moves things around and the clang-tidy comments end up in the wrong places
  3. Adding clang-format commits to .git-blame-ignore-revs keeps git blame clean but makes merge conflicts much more likely, requiring more merges to fix them + another round of CI
  4. Inconsistent versions of clang-format on dev machines and CI means formatting can flip-flop back and forth

This PR tries to fix these issues by:

  • making the CI check fail if it has to do any formatting
  • adding config so we can all get consistent versions of the tools locally and on CI
  • adding pre-commit config so that formatting can run when making commits, meaning code should always be formatted
  • no automated commits, and commits always be correctly formatted means not having to add commits to .git-blame-ignore-revs

@ZedThree
Copy link
Member Author

I'm going to make a separate PR into this one applying all the formatting rules across everything -- turns out we have a lot of trailing whitespace

dschwoerer
dschwoerer previously approved these changes Mar 11, 2026
Copy link
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 👍

@dschwoerer
Copy link
Contributor

I'm going to make a separate PR into this one applying all the formatting rules across everything -- turns out we have a lot of trailing whitespace

Maybe we should first try to reduce the number of open PRs, to avoid to many conflicts?

@ZedThree
Copy link
Member Author

I've expanded the docs on this further to make things clearer about how things work.

I've also switched clang-tidy-review to post annotations instead of comments. We'll see how that fairs

@ZedThree
Copy link
Member Author

Just a heads up once this gets merged and people start using it: you can pass git commit --no-verify to temporarily skip the pre-commit hooks. This might be necessary if switching between branches without this branch merged


.. code-block:: console

git clang-format next
Copy link
Contributor

Choose a reason for hiding this comment

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

    git clang-format origin/next

next may be an old, stale local branch. But then, origin may also be there own fork, which may also be outdated ...

@ZedThree ZedThree merged commit 82fe1de into next Mar 12, 2026
23 checks passed
@ZedThree ZedThree deleted the pre-commit-hooks branch March 12, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants