Skip to content

Add diff-scoped clang-tidy CI workflow#117

Open
pmatos wants to merge 2 commits into
mainfrom
nora/issue116
Open

Add diff-scoped clang-tidy CI workflow#117
pmatos wants to merge 2 commits into
mainfrom
nora/issue116

Conversation

@pmatos

@pmatos pmatos commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Adds .github/workflows/clang-tidy.yml, closing the most concrete gap from the
tooling audit in #116: .clang-tidy and CMAKE_EXPORT_COMPILE_COMMANDS=ON were
already configured, but no workflow ever ran the linter, so the ruleset only
affected local editors.

The job:

  • Runs on PRs to main (paths-filtered to C/C++ sources, .clang-tidy, and the
    workflow itself), mirroring the structure of scan-build.yml.
  • Configures the debug preset with clang-22/clang++-22 so the exported
    compile_commands.json carries Clang-compatible flags — configuring with GCC
    leaks GCC-only warning options that clang-tidy rejects as
    unknown-warning-option errors. Configure alone is enough (it also generates
    config.h); no build step is needed for a lint pass.
  • Lints only changed lines via clang-tidy-diff.py (git diff -U0 origin/<base>...HEAD), so the existing ruleset applies to new/modified code
    without drowning PRs in pre-existing findings from unchanged code.
  • Uses -warnings-as-errors='*' so any finding on a changed line fails the job
    via the script's exit code (verified it respects the line filter — a clean
    changed line in a file with hundreds of pre-existing warnings still passes).

Scope

#116 is an audit bundling several independent recommendations. This PR
intentionally addresses only the clang-tidy CI item so it stays small and
reviewable, hence Part of #116 rather than a closing keyword — the issue should
remain open for the remaining follow-ups:

  • ccache + FetchContent build caching
  • libFuzzer harness for the lexer/parser (+ ClusterFuzzLite)
  • actionlint + zizmor workflow linting; OpenSSF Scorecard
  • RapidCheck round-trip property tests; Catch2 v2 → v3
  • (optional) mirror the CI lint locally via a clang-tidy pre-commit hook

Part of #116

Test plan

  • cmake --preset debug (clang) generates build/debug/compile_commands.json
  • Diff harness validated locally end-to-end against LLVM 22.1.5:
    • clean changed line in a 570-warning file → exit 0
    • changed line with a lint finding → exit 1
    • no C/C++ changes → "No relevant changes found", exit 0
  • Simulated the CI step against origin/main...HEAD for this PR → no C/C++
    changes, exit 0 (job will be green)

The .clang-tidy ruleset was configured but never executed in CI; this
runs clang-tidy-diff.py over changed lines on PRs, reusing the preset's
exported compile_commands.json.
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.17%. Comparing base (f4e86f0) to head (a0492af).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #117   +/-   ##
=======================================
  Coverage   75.17%   75.17%           
=======================================
  Files          25       25           
  Lines        2908     2908           
  Branches      393      393           
=======================================
  Hits         2186     2186           
  Misses        722      722           
Flag Coverage Δ
integration 75.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a83263d86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/clang-tidy.yml
The diff glob matches src/mlir/*.cpp, but the debug preset omits src/mlir
(opt-in behind NORA_ENABLE_MLIR, MLIR/TableGen deps not installed), so those
files are not in compile_commands.json. Without -only-check-in-db,
clang-tidy-diff.py lints them with no compile command and fails on missing
mlir/IR/... and generated .inc headers, blocking any MLIR-only PR. The flag
restricts the lint to files present in the database while still linting normal
src/*.cpp changes.
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant