Sponsored Content
Skip to content

fix: Fix clang-tidy pre-commit hook to locate compile_commands.json from repo root - #7325

Merged
bthomee merged 1 commit into
developfrom
pratik/Fix-clang-tidy-script-to-locate-compile-commands
May 26, 2026
Merged

fix: Fix clang-tidy pre-commit hook to locate compile_commands.json from repo root#7325
bthomee merged 1 commit into
developfrom
pratik/Fix-clang-tidy-script-to-locate-compile-commands

Conversation

@pratikmankawde

Copy link
Copy Markdown
Contributor

High Level Overview of Change

Fix the bin/pre-commit/clang_tidy_check.py pre-commit hook so it correctly locates the repo root when searching for build/compile_commands.json. Previously the script computed the repo root as Path(__file__).parent.parent, which resolves to bin/ rather than the repository root, causing the hook to fail to find the compile database.

Also marks .github/scripts/levelization/generate.py as executable (mode change only).

Context of Change

The pre-commit hook needs compile_commands.json (generated under <repo_root>/build/) to drive clang-tidy. The previous path computation was off by one directory level, so the hook never found the compile database when invoked from the project. The fix uses git rev-parse --show-toplevel (with cwd pinned to the script's directory) to obtain the true repo root regardless of where the hook is invoked from.

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Test Plan

  • Stage a C++ file change and run the pre-commit hook with TIDY=1; confirm the hook now locates build/compile_commands.json and runs clang-tidy against staged files.
  • Run from a subdirectory of the repo to verify the git rev-parse lookup still resolves the repo root correctly.

…repo root

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@pratikmankawde pratikmankawde changed the title Fix clang-tidy pre-commit hook to locate compile_commands.json from repo root fix: Fix clang-tidy pre-commit hook to locate compile_commands.json from repo root May 26, 2026
return 0

repo_root = Path(__file__).parent.parent
repo_root = Path(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would probably put it as REPO_ROOT in the top of the file

@pratikmankawde pratikmankawde May 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, wrap it in a cached accessor, not let it be a const, change the signatures of functions needing it. It will need more efforts for you guys to review. I would rather leave be for now.

@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (a911f90) to head (63ce2ee).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7325     +/-   ##
=========================================
- Coverage     82.4%   82.3%   -0.0%     
=========================================
  Files         1011    1011             
  Lines        76330   76330             
  Branches      7318    7316      -2     
=========================================
- Hits         62874   62850     -24     
- Misses       13456   13480     +24     

see 10 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bthomee
bthomee requested a review from Copilot May 26, 2026 13:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pratikmankawde pratikmankawde 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 26, 2026
@bthomee
bthomee enabled auto-merge May 26, 2026 13:50
@bthomee
bthomee added this pull request to the merge queue May 26, 2026
Merged via the queue into develop with commit e9d885b May 26, 2026
3 checks passed
@bthomee
bthomee deleted the pratik/Fix-clang-tidy-script-to-locate-compile-commands branch May 26, 2026 14:23
@bthomee bthomee added this to the 3.2.0 milestone May 27, 2026
Kassaking7 pushed a commit to Kassaking7/rippled that referenced this pull request Jun 2, 2026
…rom repo root (XRPLF#7325)

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants