POC: Deduplicate the GMT cache download step into a composite action - #4858
Draft
seisman wants to merge 2 commits into
Draft
POC: Deduplicate the GMT cache download step into a composite action#4858seisman wants to merge 2 commits into
seisman wants to merge 2 commits into
Conversation
Six workflows (benchmarks, ci_docs, ci_doctests, ci_tests, ci_tests_dev and
ci_tests_legacy) each carried their own copy of the step that downloads the
'gmt-cache' artifact into ~/.gmt. All six were functionally identical; the only
differences were comment drift.
Replace them with a local composite action at
.github/actions/download-gmt-cache, which removes 48 lines and adds 6.
Note that composite actions do not inherit the job's 'defaults.run.shell', so the
action sets 'shell: bash -l {0}' explicitly to match the workflows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 'Ensure hyphens are not used in names of directories and Python files' step in style_checks.yaml greps every tracked directory, so '.github/actions/download-gmt-cache' would have failed CI. Rename it to 'download_gmt_cache'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Proof of concept, opened as a draft for discussion. It works and CI should pass,
but the point is to decide whether we want local composite actions in this repo at all
before going further.
What this does
Six workflows each carried their own copy of the step that downloads the
gmt-cacheartifact into
~/.gmt:benchmarks,ci_docs,ci_doctests,ci_tests,ci_tests_devandci_tests_legacy.All six were functionally identical — the only differences were comment drift
("Download files" vs "Download cached files" in
ci_tests, and a stray trailing periodin
ci_tests_legacy), which is the usual symptom of copy-paste.They now share
.github/actions/download-gmt-cache:Net -48/+6 lines in the workflows.
Things worth knowing before deciding
how CI logs read: the inner step is nested under the outer step name rather than
appearing flat in the step list.
defaults.run.shell. All six jobs useshell: bash -l {0}, so the action sets that explicitly. Without it the step wouldsilently fall back to plain
bash, which behaves differently on the Windows runnersin
ci_docs.on the same runner and needs no download (it comes from the checkout), unlike a
reusable workflow, which would add a separate job. The step it replaces takes ~33 s in
a recent
ci_testsrun, so the wrapper overhead is noise.uses: ./reads the action from the checked-out tree, so onpull_requesta forkcould alter its contents. Not a new exposure for these particular workflows — they
already execute PR code via
make install/make test, withpermissions: {}and nosecrets — but it is a difference from an inline step, which always comes from the base
branch.
zizmorreports no new findings.Possible follow-ups, deliberately not done here
Get current week number of year(4 workflows) andmake install(5 workflows) arealso duplicated, but they are 1-2 lines each, so wrapping them likely costs more in
indirection than it saves.
Idea by @seisman, and implemented by Claude Code