ci(lint): Cache necessary files for lint:cpp-static-check to skip unnecessary re-runs. - #197
Conversation
WalkthroughAdds cache restore/save steps to the code-linting-checks GitHub Actions workflow for cpp static checks with event/branch conditions, and introduces G_LINT_CLANG_TIDY_DIR in lint-tasks.yaml used as OUTPUT_DIR for clang-tidy outputs across source, tests, and examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Cache as Actions Cache
participant Lint as lint:cpp-static-check
Dev->>GH: Push / PR / Schedule
GH->>Cache: Restore cache (if event != schedule)
Cache-->>GH: Returns cached artefacts/checksums (or miss)
GH->>Lint: Run lint tasks (uses G_LINT_CLANG_TIDY_DIR for outputs)
Lint-->>GH: Lint results and generated outputs
GH->>Cache: Save cache (if event != pull_request && ref == refs/heads/main)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/code-linting-checks.yaml (1)
69-81: Fix: save step can fail on scheduled runs (empty key) and on cache hits (“already exists”).
- On schedule, the restore step is skipped, so steps.cache-restore-lint-cpp-static-check.outputs.cache-primary-key is empty; save will error (“Input required and not supplied: key”).
- With a constant key, save will also fail when the cache already exists (“Cache already exists”). Only save on cache misses and also skip on schedule.
Apply this diff to harden the save conditions and avoid empty key usage:
- - if: "'pull_request' != github.event_name && 'refs/heads/main' == github.ref" + - if: > + 'pull_request' != github.event_name + && 'schedule' != github.event_name + && 'refs/heads/main' == github.ref + && steps.cache-restore-lint-cpp-static-check.outputs.cache-hit != 'true' name: "Update lint:cpp-static-check cache" uses: "actions/cache/save@5a3ec84eff668545956fd18022155c47e93e2684" with: path: | .task/checksum/lint-cpp-static-check .task/checksum/utils-cpp-lint-clang-tidy-* build/lint-clang-tidy - key: "${{steps.cache-restore-lint-cpp-static-check.outputs.cache-primary-key}}" + key: "${{ steps.cache-restore-lint-cpp-static-check.outputs.cache-primary-key }}"This ensures:
- Save doesn’t run on schedule (no restore → no key).
- Save is skipped when the cache already exists (cache-hit == 'true'), avoiding “already exists” failures.
🧹 Nitpick comments (1)
.github/workflows/code-linting-checks.yaml (1)
54-66: Restore step is fine; consider optional restore-keys for forward-compatibility.Current restore logic and constant key meet the PR’s objective. If you later want smoother rotations, you could add restore-keys with a prefix (e.g., main-branch-ubuntu-latest-), but that’s optional given your fixed-key strategy.
Example (optional):
uses: "actions/cache/restore@5a3ec84eff668545956fd18022155c47e93e2684" with: path: | .task/checksum/lint-cpp-static-check .task/checksum/utils-cpp-lint-clang-tidy-* build/lint-clang-tidy key: "main-branch-ubuntu-latest-lint:cpp-static-check" + restore-keys: | + main-branch-ubuntu-latest-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/code-linting-checks.yaml(1 hunks)lint-tasks.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: non-storage-unit-tests (ubuntu-24.04)
- GitHub Check: non-storage-unit-tests (ubuntu-22.04)
🔇 Additional comments (3)
lint-tasks.yaml (3)
6-6: Good addition: centralised clang-tidy output directory.Introducing G_LINT_CLANG_TIDY_DIR keeps clang-tidy artefacts predictable and aligns with the cache path in the workflow.
92-103: OUTPUT_DIR wiring matches the new cache directory; verify multi-run merge behaviour.Pointing clang-tidy-find to a shared OUTPUT_DIR is fine for caching. Please confirm the utils task appends/merges outputs rather than overwriting when invoked multiple times for different ROOT_PATHS, to avoid losing the first run’s results.
If overwriting is possible, consider namespacing per invocation (e.g., subdirs per compile DB) or adjusting the utils task to aggregate.
104-113: Consistent OUTPUT_DIR across both clang-tidy-find calls looks correct.This keeps all artefacts under build/lint-clang-tidy, matching the workflow cache configuration.
I think I already did, unless u're suggesting another issue. |
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
3 participants
Description
A full-run of clang-tidy takes more than half an hour to complete for the current code base. This PR is a solution to reduce the total execution time for clang-tidy checks by adapting this PR from CLP core. It has made the following changes to make it work in Spider:
ubuntu-latest.cpp-static-checkinstead ofcheck-cpp-static-full.Checklist
breaking change.
Validation performed
Didn't try on the local branches for the actual CI execution, as the functionality has been tested in CLP core.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation