clang-format: set ColumnLimit to 0 and reformat
This commit hopefully improves the formatting of the codebase by setting ColumnLimit to 0 and hence stopping clang-format from trying to put as much stuff in one line as possible.
This change enabled us to remove most of clang-format off directives and fixed a bunch of lines that looked like this:
#define KEY \
VALUE /* comment */
Additionally, one pair of clang-format off / clang-format on had clang-format off as the second comment and hence didn't enable the formatting for the rest of the file. This commit addresses this issue as well.
Please tell me if anything in the changes seem off. If everything is fine, I will add this commit to .git-blame-ignore-revs later.
Codecov Report
Attention: Patch coverage is 98.28571% with 3 lines in your changes missing coverage. Please review.
Project coverage is 70.58%. Comparing base (
ba71c7e) to head (b8e39e5). Report is 11 commits behind head on unstable.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/t_string.c | 98.60% | 2 Missing :warning: |
| src/sentinel.c | 0.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## unstable #1045 +/- ##
============================================
- Coverage 70.62% 70.58% -0.04%
============================================
Files 114 114
Lines 61672 61684 +12
============================================
- Hits 43555 43541 -14
- Misses 18117 18143 +26
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/acl.c | 88.89% <ø> (ø) |
|
| src/ae.c | 74.90% <ø> (ø) |
|
| src/asciilogo.h | 100.00% <ø> (ø) |
|
| src/cluster_legacy.c | 85.85% <ø> (-0.48%) |
:arrow_down: |
| src/cluster_slot_stats.c | 94.04% <ø> (ø) |
|
| src/config.c | 78.69% <ø> (ø) |
|
| src/debug.c | 53.67% <ø> (ø) |
|
| src/dict.c | 97.26% <ø> (ø) |
|
| src/eval.c | 56.54% <ø> (ø) |
|
| src/expire.c | 96.42% <ø> (ø) |
|
| ... and 30 more |
Nice. Its a bit annoying that clang-format had some formatting changes somewhere between 18.1.3 and 18.1.6. Maybe that the reason for the failing CI check? The check seem to use clang-format 18.1.8.
Nice. Its a bit annoying that clang-format had some formatting changes somewhere between 18.1.3 and 18.1.6. Maybe that the reason for the failing CI check? The check seem to use clang-format 18.1.8.
Yeah, good point. I forgot to pin the clang-format version when making this change.
Basically I would suggest pinning the clang-format version both in CI and in the documentation (and for developers) because it changes within the patch versions (there will be a difference between 18.1.3 and there is a difference between 18.1.6 and 18.1.8)
Actually now I realized that the large diff in the CI was caused not by the version mismatch, but by the typo in my formatting script that caused only .h files to be reformatted automatically.
My proposal regarding the clang-format version is following: keep the things in CI the way they are right now (so, clang-format will be installed from apt.llvm.org) and observe if there are any issues about the formatting in the future.
If there are more backwards-incompatible changes with patch version of clang-format (like it was between 18.1.3 and 18.1.6), I will create a new pull request with pinning clang-format version.
Yeah I like this change too. Now thinking back the column limit is actually net negative as it forced us to spread clang-format off all over the codebase.
re - version compat. I did get into a lot of issues when switching between version 18 and 19 but I would be surprised if a patch version upgrade breaks things. that said, I see no harm pinning to a specific patch version.
Thanks for making this change, @mkmkme! Please let me know when this PR is ready for the final review.
@PingXie
but I would be surprised if a patch version upgrade breaks things
It did. I had incompatibility issues between 18.1.3 and 18.1.6 in libvalkey PR
Also I can see that "Clang Format Check" got its green mark. So @PingXie @zuiderkwast @bjosv this is now ready for a proper review :)