Sponsored Content
Skip to content

ci: Rewrite clang-tidy workflow(s) in a reusable manner - #7062

Merged
bthomee merged 13 commits into
XRPLF:developfrom
mathbunnyru:new_clang_tidy_workflow
May 4, 2026
Merged

ci: Rewrite clang-tidy workflow(s) in a reusable manner#7062
bthomee merged 13 commits into
XRPLF:developfrom
mathbunnyru:new_clang_tidy_workflow

Conversation

@mathbunnyru

@mathbunnyru mathbunnyru commented May 1, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

There are multiple improvements in this implementation:

  • determine-tidy-files.yml is moved to XRPLF/actions. The reason I kept it a workflow and not an action - there is no way in GitHub to break execution of steps and return early. So, if it were an action, every subsequent step would have been if: need_to_run.
  • We only have one workflow to call, with sane params, instead of 2, so it's much easier to follow what's going on
  • That workflow is divided better into jobs and steps. For example, instead of one huge chunk of issue creation, there are now several parts
  • Jobs now only have minimal permissions, that's why actual issue creation is separate
  • Artifacts are uploaded without a zip (it makes it easier to work with them locally)

So, from the perspective of workflows, it's a rewrite.

Context of Change

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)

Copilot AI review requested due to automatic review settings May 1, 2026 17:35

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.

Pull request overview

Reworks the clang-tidy CI implementation to use a single reusable workflow with a dedicated reusable “determine changed files” workflow and a separate job for issue creation, aiming for clearer structure and reduced permissions.

Changes:

  • Introduces .github/workflows/determine-tidy-files.yml to compute clang-tidy targets/flags based on changed files.
  • Rewrites .github/workflows/reusable-clang-tidy.yml to run clang-tidy directly (build/setup + run + artifact/issue-body generation) and split issue creation into a separate job.
  • Removes the older per-file reusable workflow and updates trigger workflows accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
.github/workflows/reusable-clang-tidy.yml Consolidates clang-tidy execution into one workflow, adds build/setup steps, artifacts, and a separate issue-creation job.
.github/workflows/reusable-clang-tidy-files.yml Deleted legacy workflow previously responsible for running clang-tidy and optionally creating issues.
.github/workflows/determine-tidy-files.yml New reusable workflow to detect changed C++/clang-tidy config files and output target lists.
.github/workflows/on-trigger.yml Removes reference to deleted reusable-clang-tidy-files.yml.
.github/workflows/on-pr.yml Removes reference to deleted reusable-clang-tidy-files.yml.

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

Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Copilot AI review requested due to automatic review settings May 1, 2026 17:44

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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.


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

Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/on-pr.yml
Comment thread .github/workflows/on-trigger.yml Outdated
Copilot AI review requested due to automatic review settings May 1, 2026 17:51

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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


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

Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Copilot AI review requested due to automatic review settings May 1, 2026 18:01

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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.


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

Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/determine-tidy-files.yml Outdated
Copilot AI review requested due to automatic review settings May 1, 2026 18:18

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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


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

Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .github/workflows/reusable-clang-tidy.yml
Comment thread .clang-tidy
@@ -6,8 +6,8 @@ Checks: "-*,
bugprone-assert-side-effect,
bugprone-bad-signal-to-kill-thread,
bugprone-bool-pointer-implicit-conversion,
bugprone-casting-through-void,

@mathbunnyru mathbunnyru May 1, 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.

There are no actual new checks here, only the file is now sorted
This helps to launch the clang tidy workflow (I also checked that without changing this file, it's not actually launched; and changing only C++ files works as well)

@mathbunnyru
mathbunnyru requested review from bthomee and godexsoft May 1, 2026 18:31
@mathbunnyru
mathbunnyru force-pushed the new_clang_tidy_workflow branch from 646fbe5 to 6c1d41d Compare May 1, 2026 18:33
Copilot AI review requested due to automatic review settings May 1, 2026 18:46
@@ -176,4 +175,4 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Fail
run: false
run: exit 1

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.

This is the incorrect type, and my editor was complaining.

Comment thread .github/workflows/reusable-clang-tidy.yml Outdated

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.

Pull request overview

Copilot reviewed 6 out of 6 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.

@mathbunnyru
mathbunnyru force-pushed the new_clang_tidy_workflow branch from 13b65fe to 18fe09c Compare May 1, 2026 19:03
Copilot AI review requested due to automatic review settings May 1, 2026 19:32
Copilot AI review requested due to automatic review settings May 1, 2026 19:44
.github/actions/generate-version/**
.github/actions/setup-conan/**
.github/scripts/strategy-matrix/**
.github/workflows/reusable-build.yml

@mathbunnyru mathbunnyru May 1, 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.

This file and a few others don't exist (Copilot correctly complained), so I removed them in this PR, since I changed this list of files

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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


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

Comment thread .github/workflows/on-trigger.yml
Comment thread .github/workflows/on-pr.yml
@codecov

codecov Bot commented May 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.1%. Comparing base (8490206) to head (ab99e1a).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7062     +/-   ##
=========================================
- Coverage     82.1%   82.1%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        76033   76033             
  Branches      7375    7368      -7     
=========================================
- Hits         62418   62411      -7     
- Misses       13615   13622      +7     

see 5 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.

Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
Comment thread .github/workflows/reusable-clang-tidy.yml Outdated
@github-actions

github-actions Bot commented May 3, 2026

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

Copilot AI review requested due to automatic review settings May 4, 2026 10:42
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

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.

Pull request overview

Copilot reviewed 6 out of 6 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.

@mathbunnyru
mathbunnyru requested a review from bthomee May 4, 2026 11:57
@mathbunnyru mathbunnyru added Trivial Simple change with minimal effect, or already tested. Only needs one approval. Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. labels May 4, 2026
@bthomee
bthomee added this pull request to the merge queue May 4, 2026
Merged via the queue into XRPLF:develop with commit d050073 May 4, 2026
3 checks passed
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
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. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants