Skip to content

feat: add cache-rebuilding workflow to go along with pre-commit workflow - #90

Open
chris11-taylor-nttd wants to merge 1 commit into
mainfrom
reusable-cache-rebuild-workflow
Open

feat: add cache-rebuilding workflow to go along with pre-commit workflow#90
chris11-taylor-nttd wants to merge 1 commit into
mainfrom
reusable-cache-rebuild-workflow

Conversation

@chris11-taylor-nttd

Copy link
Copy Markdown
Contributor

Goes along with #89, this rebuilds the caches on a given repo for asdf and pre-commit.

This is intended to be run on merge to main, so that the cache applies to main and any PRs that are created from main, decreasing actions runtime. Since caches don't persist forever, I've included workflow_dispatch as a recommended event, so that if necessary the caches can be rebuilt manually.

@bryce-lynn-nttd bryce-lynn-nttd 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.

Cleanly addresses the cache-without-save concern from PR #88. Verified the cache keys match between this workflow and PR #89's consumer side (${{ runner.os }}-pre-commit-${{ hashFiles('.pre-commit-config.yaml') }} and ${{ runner.os }}-asdf-${{ hashFiles('.tool-versions') }}), so the saved entries will hit on the restore side. actions/cache/save@27d5ce7f shares the SHA with PR #89's actions/cache/restore, which is the right pairing (both sub-actions of actions/cache).

Verifications:

  • Trigger logic is right: paths: filter on .pre-commit-config.yaml/.tool-versions avoids rebuilding when the source files (which feed the cache key) haven't changed; workflow_dispatch covers the manual override.
  • Conditional inputs (cache_precommit, cache_asdf, both default true) let consumers rebuild just one cache type.
  • Permissions minimal (contents: read).
  • Docs file thorough — especially the [!NOTE] about caches being branch-scoped.

Non-blocking:

  • actions/cache/save is a no-op when the key already exists. GitHub Actions caches are immutable by key, so workflow_dispatch against an unchanged .pre-commit-config.yaml/.tool-versions won't refresh the existing cache. To force a refresh, the existing entry must be deleted first (UI or gh cache delete). Worth a sentence in the docs noting this — the current "to rebuild only one of the two caches" example doesn't make it clear that manually triggering on unchanged configs is effectively a no-op.

  • Pair dependency with PR #89: this workflow alone is not useful — without #89's restore-side consumer, the saved entries have no purpose. Worth ensuring both land in close coordination (body already acknowledges "goes along with #89").

Generated with Claude Code (Opus 4.7)

@ben-vaughan-nttd ben-vaughan-nttd 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 PR Review

Note: This pull request review was generated by Codex.

Review Decision

APPROVE

High Findings

  • (none)

Medium Findings

  • (none)

Low Findings

  • (none)

Nitpicks

  • (none)

Previously Raised Findings Confirmed

  • The prior reviewer’s non-blocking note about actions/cache/save being a no-op when the same key already exists still applies. Codex is not claiming this as a new finding.
  • The prior reviewer’s note about coordinating this PR with PR #89 still applies. The rebuild workflow depends on the restore-side pre-commit workflow/docs being available. Codex is not claiming this as a new finding.

Open Questions / Assumptions

  • Assumption: PR #89 will be merged before or alongside this PR so the reusable-pre-commit-checks references resolve when this lands.

Required Workflow Status

  • Required workflows overall: pass
  • Failing required workflows: none
  • Pending required workflows: none

Validation Notes

  • Local checkout used: yes
  • Existing PR comments/reviews checked: yes
  • Existing findings confirmed but not claimed as new: prior cache immutability and PR #89 coordination notes remain relevant
  • Net-new blocking findings from Codex: none
  • Unresolved blocking findings from other reviewers: none
  • Required workflow checks evaluated: yes
  • Required workflow failures found: none
  • Required workflow failure investigation performed: not needed
  • Instruction files discovered: none found
  • Instruction files applied: workspace-level review guidance
  • Instruction files skipped as out-of-scope: none
  • User clarification requested for ambiguous scope: no
  • Older duplicate Codex blocking review dismissed: no
  • Checks/tests executed: pre-commit run --files .github/workflows/push-rebuild-caches.yml .github/workflows/reusable-rebuild-caches.yml docs/reusable-rebuild-caches.md; python3 scripts/check_documentation_exists.py; git diff --check

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants