⚡ Quick Guide: refactor: Enable clang-tidy `readability-identifier-naming` check by godexsoft · Pull Request #6571 · XRPLF/rippled · GitHub

godexsoft · 2026-03-17T22:41:57Z

High Level Overview of Change

This PR enables clang-tidy readability-identifier-naming changing a lot of code in the process.
Many places are hard to fix so NOLINTs were added strategically.

API Impact

No impact.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

mathbunnyru

Overall, this is great, but I left some nits.
Great improvement for the code uniformity 👍
Feel free to ignore/resolve, or whatever you think is best.

mathbunnyru · 2026-05-03T00:40:19Z

     * MPT Support: ${field['mpt_support']}
 % endif
-% if field['requirement'] == 'soeREQUIRED':
+% if field['requirement'] == 'SoeRequired':


Just in case, please check the old name no longer appears in the code

Not only in code.. it does not appear in anything at all :)

mathbunnyru · 2026-05-03T00:43:18Z

-    static state_t state;
-    std::scoped_lock const lock(state.mutex);
-    return {state.dist(state.gen), state.dist(state.gen)};
+    static StateT kSTATE;


Do we also name statics like this in Clio?
It's not the same as a constant expression, but uses the same naming pattern, would be great to make them different

Yes, and we can in newer clang-tidy but sadly we do the same in clio because in 21 we can't just yet. I will change them later when we can do it correctly for constexpr too

mathbunnyru · 2026-05-03T00:44:10Z

 template <class T>
-SharedIntrusive<T>::SharedIntrusive(SharedIntrusive&& rhs) : ptr_{rhs.unsafeExchange(nullptr)}
+SharedIntrusive<T>::SharedIntrusive(SharedIntrusive&& rhs)
+    : ptr_{std::move(rhs).unsafeExchange(nullptr)}


std::move isn't just renaming, please, check changes in this file are intended and correct

Yes these are intended. Remember that changing any code leads to new findings for already enabled checks.. so this pr does a lot more than just renaming tbh

mathbunnyru · 2026-05-03T00:44:28Z

    switch (action)
    {
-        case noop:
+        case Noop:


Maybe NoOp?

mathbunnyru · 2026-05-03T00:46:57Z

 enum LogSeverity {
-    lsINVALID = -1,  // used to indicate an invalid severity
-    lsTRACE = 0,     // Very low-level progress information, details inside
+    LsInvalid = -1,  // used to indicate an invalid severity


These should be LSInvalid and so on, because L and S are parts of different words

I'll rename for now but really we should remove this deprecated enum and use beast::severities::Severity instead as the comment suggested.

mathbunnyru · 2026-05-03T00:50:14Z


-    constexpr static int cMinOffset = -96;
-    constexpr static int cMaxOffset = 80;
+    constexpr static int kC_MIN_OFFSET = -96;


Probably should be just kMIN_OFFSET.
Please, fix here and search for similar pattern

mathbunnyru · 2026-05-03T00:51:02Z

    template <class... Args>
    void
-    emplace_back(Args&&... args);
+    emplaceBack(Args&&... args);


I don't have an opinion about places like this, to be honest.
But it might break some template code which checks for emplace_back to be available, and hoping for either std::vector or STArray

I actually kept some of the ones that you describe. Not sure this one is tbh., will check. I mostly went by code compiling and tests passing

For this one i see no evidence that we used it in some sort of concept or similar so i think it's fine. The name itself is also fine as it conforms to clang-tidy settings. For things that definitely needed to stay i have systematically been adding exceptions but these are mostly free standing functions.

mathbunnyru · 2026-05-03T00:54:06Z

@@ -1,53 +0,0 @@
-#pragma once


Is this file deletion on purpose?

Hell no. How did you even spot that? 😅

Ok so this and some other file was deleted because it's not used anywhere in the codebase. So this is fine 👍

Hell no. How did you even spot that? 😅

From the phone it looked like it was deleting just the '#pragma once' line so i was surprised :)

mathbunnyru · 2026-05-03T00:57:36Z

+  readability-identifier-naming.UnionCase: CamelCase
+  readability-identifier-naming.EnumCase: CamelCase


Maybe let's keep the entries sorted?

They are actually grouped logically, not alphabetically. And it's the same in Clio so i think it's fine as is for now

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

bthomee · 2026-05-02T15:08:27Z

  misc-include-cleaner.IgnoreHeaders: ".*/(detail|impl)/.*;.*fwd\\.h(pp)?;time.h;stdlib.h;sqlite3.h;netinet/in\\.h;sys/resource\\.h;sys/sysinfo\\.h;linux/sysinfo\\.h;__chrono/.*;bits/.*;_abort\\.h;boost/uuid/uuid_hash.hpp;boost/beast/core/flat_buffer\\.hpp;boost/beast/http/field\\.hpp;boost/beast/http/dynamic_body\\.hpp;boost/beast/http/message\\.hpp;boost/beast/http/read\\.hpp;boost/beast/http/write\\.hpp;openssl/obj_mac\\.h"
-#
-HeaderFilterRegex: '^.*/(test|xrpl|xrpld)/.*\.(h|hpp)$'
+HeaderFilterRegex: '^.*/(test|xrpl|xrpld)/.*\.(h|hpp|ipp)$'


Should tests be added here too (the one from src/tests/; the tests/ can probably be skipped)?

There are currently no header files in it, but as tests are migrated from src/test/ to src/tests/they might start showing up.

I think in the future possibly if we need them 👍

)

…PLF#6571)

godexsoft added 6 commits March 17, 2026 18:29

Rename m_smth -> smth_

274e969

Rename mSmth -> smth_

8c2ae98

Fix compilation after renaming stuff

5cdd06e

A few more

13abc8c

Add clang-tidy settings

ce5f7cf

Merge branch 'develop' into refactor/naming-variables-style

ce9ff71

godexsoft added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Mar 17, 2026

godexsoft commented Mar 17, 2026

View reviewed changes

Comment thread include/xrpl/basics/spinlock.h Outdated

godexsoft added 12 commits March 17, 2026 23:13

Fix ci issues

b86ad07

Fix more

515cf63

Fix more

c9b3cda

Fix more ci failures

3ff79da

Fix remaining debug build issues

b80b79f

More changes from identifier-naming

ba6d457

Change constants etc. with clang-tidy

0e8c017

More stuff

1de7a72

Fix some enumerators to match style

0cbec99

Manual renames

34a071c

Merge branch 'develop' into refactor/naming-variables-style

651b640

Fix remaining issues after merge

3231791

godexsoft changed the title ~~refactor: Variable naming style~~ refactor: Enable clang-tidy readability-identifier-naming Mar 18, 2026

godexsoft added 2 commits March 18, 2026 23:01

Fix some tests and issues found in ci

6502d58

Multiple fixes of naming issues that clang-tidy could not fix itself

1dff3c4

godexsoft mentioned this pull request Mar 19, 2026

refactor: Enable more clang-tidy readability checks #6595

Merged

4 tasks

godexsoft added 6 commits March 20, 2026 14:37

Fix many more issues found via LSP

b05d556

Fix unittests after refactoring

cdecae8

Merge branch 'develop' into refactor/naming-variables-style

f042fcb

Fix more issues visible only through CI

84670a8

Merge branch 'develop' into refactor/naming-variables-style

1ef2ac5

Run clang-tidy with fixes again

217a866

Copilot AI review requested due to automatic review settings May 2, 2026 17:42

Copilot AI reviewed May 2, 2026

godexsoft added 3 commits May 2, 2026 19:17

Debug builds for clang-tidy

878d9b7

Fix a few more findings

4d9dce7

Fix more claude findings

1aca53e

Copilot AI review requested due to automatic review settings May 2, 2026 19:50

Copilot AI reviewed May 2, 2026

godexsoft added 2 commits May 2, 2026 21:25

Revert mako comment change

e42c55e

Fix mako comment again and regen autogen stuff

72bb687

Copilot AI review requested due to automatic review settings May 2, 2026 20:31

Copilot AI reviewed May 2, 2026

mathbunnyru approved these changes May 3, 2026

View reviewed changes

mathbunnyru reviewed May 3, 2026

View reviewed changes

godexsoft added 2 commits May 3, 2026 02:31

Fix review comments

8e7e29e

Fix a few more findings

026d14b

Copilot AI review requested due to automatic review settings May 3, 2026 01:40

Copilot AI reviewed May 3, 2026

godexsoft added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 3, 2026

bthomee approved these changes May 3, 2026

View reviewed changes

bthomee changed the title ~~refactor: Enable clang-tidy readability-identifier-naming~~ refactor: Enable clang-tidy readability-identifier-naming check May 3, 2026

bthomee added this pull request to the merge queue May 3, 2026

Merged via the queue into XRPLF:develop with commit 8995564 May 3, 2026
3 checks passed

godexsoft mentioned this pull request May 3, 2026

chore: Ignore identifier-naming update in git blame #7066

Merged

Copilot AI mentioned this pull request May 4, 2026

refactor: Replace boost::filesystem with std::filesystem across the codebase #7012

Merged

1 task

bthomee pushed a commit that referenced this pull request May 4, 2026

refactor: Enable clang-tidy readability-identifier-naming check (#6571

3d06263

)

bthomee mentioned this pull request May 6, 2026

fix: Restore clang-tidy change to section name in config #7091

Merged

godexsoft mentioned this pull request May 12, 2026

refactor: Rename static constants #7120

Merged

mvadari added this to the 3.2.0 milestone May 20, 2026

beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026

refactor: Enable clang-tidy readability-identifier-naming check (XR…

367c871

…PLF#6571)

		readability-identifier-naming.UnionCase: CamelCase
		readability-identifier-naming.EnumCase: CamelCase

Uh oh!

Conversation

godexsoft commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

API Impact

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mathbunnyru left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathbunnyru May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathbunnyru May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathbunnyru May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathbunnyru May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

godexsoft commented Mar 17, 2026 •

edited

Loading

mathbunnyru May 3, 2026 •

edited

Loading

mathbunnyru May 3, 2026 •

edited

Loading

mathbunnyru May 3, 2026 •

edited

Loading

mathbunnyru May 3, 2026 •

edited

Loading