-
Notifications
You must be signed in to change notification settings - Fork 9
fix(ci): Correctly update and restore cache for lint:cpp-static-check's generated files .
#260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughReplaced a fixed cache key with a descriptive prefix and a hash-derived save key for the C++ static lint step in the GitHub Actions workflow; added notes about prefix-based restore, hashFiles() usage, and that scheduled runs do not restore the lint cache so lint runs on all files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Areas requiring attention:
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2024-10-23T02:23:12.161ZApplied to files:
⏰ 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)
🔇 Additional comments (3)
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. Comment |
LinZhihao-723
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Adding more comment mirrored from y-scope/clp#1430.
- Adding a note to explain how the cache key works, as the prefix match isn't obvious.
Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>
Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>
lint:cpp-static-check's generated files .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/code-linting-checks.yaml:
- Around line 69-76: Fix the typo in the comment above the cache key: change
"prefix.y." to "prefix." in the comment block that documents the cache key (the
block referencing key: "lint:check-cpp-static-on-ubuntu-latest") so the sentence
reads "Use a prefix so the cache action can match and restore the most recent
cache entry that shares that prefix.".
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/code-linting-checks.yaml
⏰ 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: non-storage-unit-tests (ubuntu-24.04)
- GitHub Check: non-storage-unit-tests (ubuntu-22.04)
- GitHub Check: lint
🔇 Additional comments (2)
.github/workflows/code-linting-checks.yaml (2)
55-59: Clear documentation of scheduled run behavior.The updated comment effectively explains why scheduled runs skip cache restoration, helping future maintainers understand the periodic full-lint strategy.
91-99: Verify hashFiles behavior with directory glob patterns.The key construction uses
hashFiles('build/lint-clang-tidy/**/*')to include all files under that directory. While GitHub Actions' hashFiles supports glob patterns, verify that:
- The directory glob correctly captures all intended files
- hashFiles behaves as expected when the directory contains many files or subdirectories
- The hash remains stable across runs when cache contents are identical
Consider testing this by:
- Checking that new cache entries are created after lint task modifies files
- Confirming cache is restored correctly on subsequent runs
- Monitoring for any hash collisions or empty hash values
Based on the PR description, initial validation shows this working correctly (cache created on push, restored on subsequent push, runtime reduced from 54 to 16 minutes).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
#197 added caching of
lint:cpp-static-checkin GitHub workflow, which does not correctly insert new caches. The cache created are immutable, but we save cache using the same key, and the cache save failed. Thus, the GitHub workflow are using an obsolete cache that are create.This PR fixes this issue by appending the has of the cache content to the key. When restoring the cache, we use a prefix of that key, and actions/cache/restore will automatically select the newest entry.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.