style: Add cmake-format pre-commit hook#6279
Merged
bthomee merged 15 commits intoXRPLF:developfrom Jan 29, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6279 +/- ##
=========================================
- Coverage 79.9% 79.8% -0.0%
=========================================
Files 839 839
Lines 65528 65528
Branches 7274 7273 -1
=========================================
- Hits 52329 52324 -5
- Misses 13199 13204 +5 see 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
bthomee
reviewed
Jan 27, 2026
CMakeLists.txt
Outdated
| endif () | ||
|
|
||
| ### | ||
| # |
Collaborator
There was a problem hiding this comment.
Maybe this section "separator" character should just be removed?
.cmake-format.yaml
Outdated
Comment on lines
+6
to
+13
| foo: | ||
| flags: | ||
| - BAR | ||
| - BAZ | ||
| kwargs: | ||
| HEADERS: "*" | ||
| SOURCES: "*" | ||
| DEPENDS: "*" |
Collaborator
There was a problem hiding this comment.
Is this an actual sub-command that can be provided, and if so what does it do?
Contributor
Author
There was a problem hiding this comment.
I think it was an example from somewhere, and command foo doesn't make sense, removed it
| disable: false | ||
| _help_line_width: | ||
| - How wide to allow formatted cmake files | ||
| line_width: 120 |
Collaborator
There was a problem hiding this comment.
Although the line width adopted throughout the code somewhat varies from file to file and from moment to moment, it generally seems to be 80 characters. I'd suggest to therefore set this value to 80. Also in .clang-format there is ColumnLimit: 80.
Co-authored-by: Bart <bthomee@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a cmake-format pre-commit hook to automatically format CMake files according to a consistent style guide. The configuration matches the style used in Clio. The PR applies the formatting to most CMake files in the repository, with some files explicitly excluded as exceptions to be handled in a future PR.
Changes:
- Added cmake-format pre-commit hook with configuration matching Clio's style
- Reformatted multiple CMake files with consistent spacing, indentation, and line wrapping
- Added cmake-format configuration terms to cspell dictionary
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .pre-commit-config.yaml | Adds cmake-format hook with excluded files list |
| .cmake-format.yaml | Comprehensive cmake-format configuration with 120-char line width |
| .config/cspell.config.yaml | Adds legitimate terms from cmake-format config (hwrap, kwarg, kwargs, pargs) |
| src/tests/libxrpl/CMakeLists.txt | Reformats if/endif spacing |
| cmake/deps/Boost.cmake | Reformats find_package, target_link_libraries, and if/endif blocks |
| cmake/create_symbolic_link.cmake | Reformats function and if/endif spacing, condenses multi-line comment |
| cmake/XrplVersion.cmake | Reformats foreach and if/endif spacing |
| cmake/XrplValidatorKeys.cmake | Reformats option, function calls, and FetchContent_Declare |
| cmake/XrplSettings.cmake | Reformats multiple option, if/endif, and set commands with improved comment wrapping |
| cmake/XrplSanity.cmake | Reformats set, if/endif, and message calls with improved multi-line message formatting |
| cmake/XrplSanitizers.cmake | Reformats extensive if/endif blocks, foreach loops, and message calls |
| cmake/XrplInterface.cmake | Reformats target configuration commands and if/endif blocks |
| cmake/XrplCov.cmake | Reformats if/endif and function calls with improved argument layout |
| cmake/XrplConfig.cmake | Reformats include, set, find_dependency, and if/endif blocks |
| cmake/XrplCompiler.cmake | Reformats extensive target_compile_options, target_link_libraries, and if/endif blocks |
| cmake/XrplAddTest.cmake | Reformats function definition and file globbing |
| cmake/CompilationEnv.cmake | Reformats multi-line comments and if/endif blocks |
| cmake/Ccache.cmake | Reformats if/endif blocks and multi-line comments |
| cmake/CMakeFuncs.cmake | Reformats macro/function definitions and execute_process calls |
| CMakeLists.txt | Reformats if/endif blocks, include statements, and target_link_libraries calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+36
to
+44
| cmake/CodeCoverage.cmake| | ||
| cmake/XrplCore.cmake| | ||
| cmake/XrplDocs.cmake| | ||
| cmake/XrplInstall.cmake| | ||
| cmake/add_module.cmake| | ||
| cmake/isolate_headers.cmake| | ||
| cmake/target_link_modules.cmake| | ||
| cmake/target_protobuf_sources.cmake| | ||
| tests/conan/CMakeLists.txt |
Contributor
There was a problem hiding this comment.
I don't think we should exclude files. Rather just update the formatting settings(if the end format is not looing good as per the PR description). Everything should follow the same format. That's the whole purpose of adding this formatter, isn't it?
Contributor
Author
There was a problem hiding this comment.
I explained this choice in the description.
I will deal with files which require manual adjustments in a later PR - this way it's easier to implement, and easier to review.
cmake/CMakeFuncs.cmake
Outdated
| OUTPUT_STRIP_TRAILING_WHITESPACE | ||
| ERROR_QUIET) | ||
| set(_branch "") | ||
| execute_process( |
Contributor
There was a problem hiding this comment.
execute_process(COMMAND ${GIT_EXECUTABLE} "rev-parse" "--abbrev-ref" "HEAD"
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
RESULT_VARIABLE _git_exit_code
OUTPUT_VARIABLE _temp_branch
OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET)
Can we achieve this formatting? Looks much cleaner. Possible now since we have 120 column size.
Contributor
Author
There was a problem hiding this comment.
Was able to do it:
- dangle_parens: true
+ dangle_parens: false
- min_prefix_chars: 4
+ min_prefix_chars: 18
It changed many places though
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 17 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bthomee
approved these changes
Jan 29, 2026
bthomee
pushed a commit
that referenced
this pull request
Feb 3, 2026
This change adds `cmake-format` as. a pre-commit hook. The style file closely matches that in Clio, and they will be made to be equivalent over time. For now, some files have been excluded, as those need some manual adjustments, which will be done in future changes.
gregtatcam
added a commit
to gregtatcam/rippled
that referenced
this pull request
Mar 7, 2026
* chore: Set ColumnLimit to 120 in clang-format (XRPLF#6288) This change updates the ColumnLimit from 80 to 120, and applies clang-format to reformat the code. * chore: Remove unnecessary `boost::system` requirement from conanfile (XRPLF#6290) * chore: Add cmake-format pre-commit hook (XRPLF#6279) This change adds `cmake-format` as. a pre-commit hook. The style file closely matches that in Clio, and they will be made to be equivalent over time. For now, some files have been excluded, as those need some manual adjustments, which will be done in future changes. * chore: Format all cmake files without comments (XRPLF#6294) * ci: Update hashes of XRPLF/actions (XRPLF#6316) This updates the hashes of all XRPLF/actions to their latest versions. * chore: Add upper-case match for ARM64 in CompilationEnv (XRPLF#6315) * fix: Restore config changes that broke standalone mode (XRPLF#6301) When support was added for `xrpld.cfg` in addition to `rippled.cfg` in XRPLF#6098, as part of an effort to rename occurrences of ripple(d) to xrpl(d), the clearing and creation of the data directory were modified for what, at the time, seemed to result in an equivalent code flow. This has turned out to not be true, which is why this change restores two modifications to `Config.cpp` that currently break running the binary in standalone mode. * docs: Update API changelog, add APIv2+APIv3 version documentation (XRPLF#6308) This change cleans up the `API-CHANGELOG.md` file. It moves the version-specific documentation to other files and fleshes out the changelog with all the API-related changes in each version. * chore: Add .zed editor config directory to .gitignore (XRPLF#6317) This change adds the project configuration directory to `.gitignore` for the `zed` editor. As per the [documentation](https://zed.dev/docs/remote-development?highlight=.zed#zed-settings), the project configuration files are stored in the `.zed` directory at the project root dir. * fix: Deletes expired NFToken offers from ledger (XRPLF#5707) This change introduces the `fixExpiredNFTokenOfferRemoval` amendment that allows expired offers to pass through `preclaim()` and be deleted in `doApply()`, following the same pattern used for expired credentials. * refactor: Add ServiceRegistry to help modularization (XRPLF#6222) Currently we're passing the `Application` object around, whereby the `Application` class acts more like a service registry that gives other classes access to other services. In order to allow modularization, we should replace `Application` with a service registry class so that modules depending on `Application` for other services can be moved easily. This change adds the `ServiceRegistry` class. * chore: Remove unity builds (XRPLF#6300) Unity builds were intended to speed up builds, by bundling multiple files into compilation units. However, now that ccache is available on all platforms, there is no need for unity builds anymore, as ccache stores compiled individual build objects for reuse. This change therefore removes the ability to make unity builds. * refactor: Replace include guards by '#pragma once' (XRPLF#6322) This change replaces all include guards in the `src/` and `include/` directories by `#pragma once`. * chore: Remove unnecessary script (XRPLF#6326) * chore: Update secp256k1 and openssl (XRPLF#6327) * fix typo in LendingHelpers unit-test (XRPLF#6215) * fix: Increment sequence when accepting new manifests (XRPLF#6059) The `ManifestCache::applyManifest` function was returning early without incrementing `seq_`. `OverlayImpl `uses this sequence to identify/invalidate a cached `TMManifests` message, which is exchanged with peers on connection. Depending on network size, startup sequencing, and topology, this can cause syncing issues. This change therefore increments `seq_` when a new manifest is accepted. * refactor: Update secp256k1 to 0.7.1 (XRPLF#6331) The latest secp256k1 release, 0.7.1, contains bug fixes that we may benefit from, see https://github.com/bitcoin-core/secp256k1/blob/master/CHANGELOG.md. * chore: Restore unity builds (XRPLF#6328) In certain cases, such as when modifying headers used by many compilation units, performing a unity build is slower than when performing a regular build with `ccache` enabled. There is also a benefit to a unity build in that it can detect things such as macro redefinitions within the group of files that are compiled together as a unit. This change therefore restores the ability to perform unity builds. However, instead of running every configuration with and without unity enabled, it is now only enabled for a single configuration to maintain lower computational use. As part of restoring the code, it became clear that currently two configurations have coverage enabled, since the check doesn't focus specifically on Debian Bookworm so it also applies to Debian Trixie. This has been fixed too in this change. * perf: Remove unnecessary caches (XRPLF#5439) This change removes the cache in `DatabaseNodeImp` and simplifies the caching logic in `SHAMapStoreImp`. As NuDB and RocksDB internally already use caches, additional caches in the code are not very valuable or may even be unnecessary, as also confirmed during preliminary performance analyses. * chore: Remove CODEOWNERS (XRPLF#6337) * test: Add file and line location to Env (XRPLF#6276) This change uses `std::source_location` to output the file and line location of the call that triggered a failed transaction. * refactor: Fix spelling issues in tests (XRPLF#6199) This change removes the `src/tests` exception from the `cspell` config and fixes all the issues that arise as a result. No functionality/test change. * refactor: Change main thread name to `xrpld-main` (XRPLF#6336) This change builds on the thread-renaming PR (XRPLF#6212), by renaming the main thread name to reduce ambiguity in performance monitoring tools. * fix: Update invariant checks for Permissioned Domains (XRPLF#6134) * refactor: Modularize WalletDB and Manifest (XRPLF#6223) This change modularizes the `WalletDB` and `Manifest`. Note that the wallet db has nothing to do with account wallets and it stores node configuration, which is why it depends on the manifest code. * refactor: Modularize RelationalDB (XRPLF#6224) The rdb module was not properly designed, which is fixed in this change. The module had three classes: 1) The abstract class `RelationalDB`. 2) The abstract class `SQLiteDatabase`, which inherited from `RelationalDB` and added some pure virtual methods. 3) The concrete class `SQLiteDatabaseImp`, which inherited from `SQLiteDatabase` and implemented all methods. The updated code simplifies this as follows: * The `SQLiteDatabaseImp` has become `SQLiteDatabase`, and * The former `SQLiteDatabase `has merged with `RelationalDatabase`. * chore: Fix `gcov` lib coverage build failure on macOS (XRPLF#6350) For coverage builds, we try to link against the `gcov` library (specific to the environment). But as macOS doesn't have this library and thus doesn't have the coverage tools to generate reports, the coverage builds on that platform were failing on linking. We actually don't need to explicitly force this linking, as the `CodeCoverage` file already has correct detection logic (currently on lines 177-193), which is invoked when the `--coverage` flag is provided: * AppleClang: Uses `xcrun -f llvm-cov` to set `GCOV_TOOL="llvm-cov gcov"`. * Clang: Finds `llvm-cov` to set `GCOV_TOOL="llvm-cov gcov"`. * GCC: Finds `gcov` to set `GCOV_TOOL="gcov"`. The `GCOV_TOOL` is then passed to `gcovr` on line 416, so the correct tool is used for processing coverage data. This change therefore removes the `gcov` suffix from lines 473 and 475 in the `CodeCoverage.cmake` file. * refactor: Modularize the NetworkOPs interface (XRPLF#6225) This change moves the NetworkOPs interface into `libxrpl` and it leaves its implementation in `xrpld`. * chore: Fix minor issues in comments (XRPLF#6346) * refactor: Modularize `HashRouter`, `Conditions`, and `OrderBookDB` (XRPLF#6226) This change modularizes additional components by moving code to `libxrpl`. * chore: Update clang-format to 21.1.8 (XRPLF#6352) * refactor: Decouple app/tx from `Application` and `Config` (XRPLF#6227) This change decouples app/tx from `Application` and `Config` to clear the way to moving transactors to `libxrpl`. * refactor: Modularize app/tx (XRPLF#6228) * ci: Add clang tidy workflow to ci (XRPLF#6369) * chore: Set cmake-format width to 100 (XRPLF#6386) * chore: Set clang-format width to 100 in config file (XRPLF#6387) * chore: Apply clang-format width 100 (XRPLF#6387) * Fix tautological assertion (XRPLF#6393) * ci: Add dependabot config (XRPLF#6379) * ci: Build docs in PRs and in private repos (XRPLF#6400) * ci: [DEPENDABOT] bump codecov/codecov-action from 5.4.3 to 5.5.2 (XRPLF#6398) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.4.3 to 5.5.2. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@18283e0...671740a) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 5.5.2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix merge conflicts, format, spelling, replace all include guards by #pragma once * Fix non-unity build --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com> Co-authored-by: Ed Hennis <ed@ripple.com> Co-authored-by: Bart <bthomee@users.noreply.github.com> Co-authored-by: Mayukha Vadari <mvadari@ripple.com> Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Jingchen <a1q123456@users.noreply.github.com> Co-authored-by: Niq Dudfield <ndudfield@gmail.com> Co-authored-by: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Co-authored-by: Olek <115580134+oleks-rip@users.noreply.github.com> Co-authored-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Co-authored-by: nuxtreact <nuxtreact@outlook.com> Co-authored-by: Sergey Kuznetsov <skuznetsov@ripple.com> Co-authored-by: Ayaz Salikhov <asalikhov@ripple.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
I add
cmake-formatpre-commit hook.The style file is exactly like in Clio (so I would probably argue not to change it).
For now, I've added files where applying cmake-format doesn't look too bad, and added other files as exceptions - manually adjusting these files is a bit time-consuming, so I would probably do it separately, but we can benefit from cmake-format for most files already.
Context of Change
Type of Change
.gitignore, formatting, dropping support for older tooling)API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)