Repository navigation
Formatting C/CPP files - #1623
Conversation
|
Looks like a build failure. |
|
All five failed jobs share one cause: include sorting exposed an invalid The PR moves #ifndef _POSIX_C_SOURCE
#define _POSIX_C_SOURCE
#endifBecause the macro has no value, comparisons in system headers become invalid. The logs show:
The docs and canary builds hit the same compilation failure. Subsequent I reproduced this locally using the PR’s exact files. Removing the empty macro definition makes the C syntax check pass; restoring the original include order also passes. Removing the invalid definition is the cleaner fix, subject to full CI validation. I haven’t changed your checkout. |
|
Hi @schaubh. Yup, just saw this also locally. I will remove the macro definition and then push to see how CI behaves. |
|
sphinx is treating warnings as errors. Logs show:
Fix is described in the warning. I'll try fix and then re-push |
|
|
a62f557 to
b1f0065
Compare
|
when rebasing i re-encountered the posix define issue, and the fix for that got swallowed into another commit. I will make that change a separate commit so that it is more visible in the git history |
b1f0065 to
8aa38d0
Compare
Simply called ``` pre-commit run clang-format --all-files ```
…OT all c/cpp code
Set BreakAfterReturnType to Automatic and reformat affected files to improve readability. Long signatures can still wrap, and include sorting remains enabled.
…dentation Use IndentOnly to preserve comment line breaks. Set continuation and constructor initializer indentation to four spaces and reformat affected C/C++ files. Retain two-space indentation for class access labels.
Replace comma expressions with condition && message in the translating and spinning body accessors, preserving bounds checks and diagnostics. Verified both Debug targets rebuild without warnings and both getter tests pass.
8aa38d0 to
06a5757
Compare
|
I pushed a few changes to the formatting guidelines and small fixes. CI tests are now running. Overall looks very good. I built in Release mode and Debug mode to ensure there were no issues. |
2 participants
Description
This PR has a massive diff so it's critical to review by commit.
The first two commits (777ed71 and 3fe58ce) do the bulk of the adjustments to pre-commit and CI to incorporate clang formatting.
The third commit (230c7b8) clang formats all C/CPP files. No functional changes are made, just formatting.
The fourth commit (197792c) ensures that clang format in the CI occurs only for files that have since been changed by a PR. This avoids checking all C/CPP files for format correctness in the CI.
The fifth commit (e5afd67) updates the documentation accordingly.
Verification
Only formatting changes were made. No tests needed.
Documentation
Updated contributing and coding guidelines accordingly.
Future work
Might make sense to add python formatting as well for any code that is not generated by SWIG.