Format the codebase with clang-format#12957
Conversation
github-actions
bot
added
documentation
new-cli
grahamc
force-pushed
the
check-format
branch
2 times, most recently
from
577f0b8 to
2bb5def
Compare
roberth
left a comment
roberth
left a comment
There was a problem hiding this comment.
Not meaning to delay the bandaid ripping, but it does seem like an opportunity to rip off a proverbial speck of dirt along with it :)
grahamc
force-pushed
the
check-format
branch
2 times, most recently
from
a4c85cf to
d3d1c7c
Compare
In nixpkgs we have some tooling to fix existing pull requests after we reformatted everything with nixfmt. Maybe we can adapt the approach. I can also recommend to install https://mergiraf.org/ into your git locally as it is much more robust against whitespace changes than the git merge algorithm.
Probably. Otherwise our backport action becomes useless.
I believe .git-blame-ignore-revs should prevent this? |
grahamc
force-pushed
the
check-format
branch
5 times, most recently
from
d0aaa87 to
b242452
Compare
|
+1 on the things @Mic92 said. I want to try out those tools either way!
In addition to what @Mic92 said: Still, it might make sense to do a dedicated push trying to get the open PR count down first. Yes, that blocks this, but it is a good chore to do in its own right! So I don't mind the delay so much for that reason. |
|
The result of the last meeting that we all are for this change and we just need to figure out how we migrate old pull requests. |
|
FYI lix has a .clang-format file here, which you probably want to diff with and check if there's anything you want to change: https://git.lix.systems/lix-project/lix/src/branch/main/.clang-format We already went through the bikeshedding how to get the formatter to match existing formatting reasonably. |
|
This PR is using the existing .clang-format in Nix's repo fwiw |
|
Having our formatting be in sync does help cherry-pick things back and forth. |
grahamc
force-pushed
the
check-format
branch
4 times, most recently
from
d557902 to
e63cbd5
Compare
|
Hey @Ericson2314 I'm open to whatever the Nix team wants to do. In the meantime, I'm continuing to refresh this PR. Like I mentioned above, I think the current situation of inconsistently applied formatting is sort of the local minima of outcomes. Do you think that is true? |
* It is tough to contribute to a project that doesn't use a formatter, * It is extra hard to contribute to a project which has configured the formatter, but ignores it for some files * Code formatting makes it harder to hide obscure / weird bugs by accident or on purpose, Let's rip the bandaid off? Note that PRs currently in flight should be able to be merged relatively easily by applying `clang-format` to their tip prior to merge.
6 participants
Motivation
Let's rip the bandaid off?
Note that PRs currently in flight should be able to be merged relatively easily by applying
clang-formatto their tip prior to merge.I would also expect it to be not too hard to apply patch backports if the backport targets are similarly formatted with clang-format.
Context
Implementation strategy requires a bit of effort on each branch in active maintenance:
Once that is done, backports should be easy to apply.
Maintainers can focus on only the change in the first commit, and optionally completely regenerate the second commit to ensure it wasn't tampered with.
This has been done somewhat: new files must be formatted. However, that makes it challenging to contribute with editor integration for code formatting.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.