feat: add cache-save-if input to control cache saving#4
Conversation
Allow users to disable cache saving while still restoring caches. This is useful for matrix jobs or limiting cache writes to specific branches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I need this to cache only on main branch. |
|
oh wait this thing always read cache |
There was a problem hiding this comment.
Pull request overview
This PR adds a new GitHub Action input, cache-save-if, to control whether dependency caches are saved during the post phase (while still allowing cache restore during the main phase when caching is enabled).
Changes:
- Added
cacheSaveIfto theInputstype and wired it into input parsing (cache-save-if). - Updated post-phase behavior to only call
saveCache()when bothcacheandcacheSaveIfare true. - Updated
action.ymland rebuiltdist/index.mjsto include the new input and logic.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Extends the Inputs interface with the new cacheSaveIf boolean. |
| src/inputs.ts | Reads the new cache-save-if boolean input into Inputs. |
| src/inputs.test.ts | Updates expected input shape to include cacheSaveIf. |
| src/index.ts | Gates cache saving in runPost() on inputs.cache && inputs.cacheSaveIf. |
| dist/index.mjs | Rebuilt bundle reflecting the new input and save gating. |
| action.yml | Declares cache-save-if input (default true) and documents behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| @@ -54,7 +54,7 @@ async function printViteVersion(): Promise<void> { | |||
|
|
|||
| async function runPost(inputs: Inputs): Promise<void> { | |||
| // Save cache if enabled | |||
There was a problem hiding this comment.
The comment says cache is saved “if enabled”, but saving is now additionally gated by inputs.cacheSaveIf. Please update the comment to reflect the actual condition (cache enabled AND cache-save-if true) so the post-phase behavior is clear.
| // Save cache if enabled | |
| // Save cache if enabled and the cache-save-if condition is true |
| runInstall: [], | ||
| cache: false, | ||
| cacheDependencyPath: undefined, | ||
| cacheSaveIf: false, | ||
| }); |
There was a problem hiding this comment.
This test sets getBooleanInput to always return false, so it won’t catch regressions where getInputs() reads the wrong boolean input name (e.g. a typo in cache-save-if). Consider adding a focused test that makes getBooleanInput return true only for "cache-save-if" (and/or asserts it was called with that name) and then verifies inputs.cacheSaveIf is true.
Summary
cache-save-ifinput (default"true") that controls whether the cache is saved in the post phasefalse, cache is only restored but never saved — useful for matrix jobs or limiting cache writes to specific branchessaveCache()call on bothinputs.cacheandinputs.cacheSaveIf🤖 Generated with Claude Code