pathogen-repo-ci: Don't run workflow build steps if they're not going to do anything #96
Conversation
This file contains 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
tsibley
force-pushed
the
trs/pathogen-repo-ci/skip-it-skip-it
branch
2 times, most recently
from
6917e28
to
6c28c6b
Compare
7 tasks
genehack
force-pushed
the
bring-tha-noise-92
branch
2 times, most recently
from
1bad99e
to
df99e08
Compare
|
I don't understand why it needs to be generated from a template? What's going on with that? |
genehack
force-pushed
the
bring-tha-noise-92
branch
from
df99e08
to
fea28cb
Compare
joverlee521
approved these changes
Jun 13, 2024
joverlee521
reviewed
Jun 13, 2024
tsibley
added a commit
that referenced
this pull request
Jun 14, 2024
…rkflow summary A good suggestion by @joverlee521 in review.¹ ¹ <#96 (comment)>
… to do anything Instead of checking for whether to do anything or not _inside_ each build step individually, move the check to the step's condition. This will make the logs a lot clearer and mean the GitHub workflow metadata stays more in sync with reality. I've used hashFiles() to check for file existence—it returns the empty string, a falsey value, when there are no matching files—but we could swap that out for using the output of a prior setup step that runs some shell to determine what to run and what to skip. Doing so might make more sense if the conditional becomes more complicated or we want to do more detailed reporting on _why_ steps were skipped or not. With the changes, the internal-to-this-workflow action, run-nextstrain-ci-build, is no longer that useful and so I've removed it in favor of inlining things. I think this is an improvement for readability. Related-to: <#95 (review)>
…ed input YAML This allows us to author the workflow using YAML anchors/references (especially with merge keys) since GitHub Actions doesn't otherwise support those YAML features. The lack of support is a shame because GitHub Actions workflows can be very repetitive and anchors/references/merges are a decent solution to that. I'm about to add substantial conceptual replication that we won't want to maintain concretely replicated in the file. Since the generated file must be checked in, a CI step ensures the generated file matches the authored file and the generated file is excluded from git diffs by default. An optional local pre-commit hook is also available for making sure you craft commits that won't run afoul of the CI check later. Or you can, as necessary, run `make` manually before committing. (Commit messaged based on edd3290.)
This lets us more easily test it in development. (Commit based on 1f41bea.)
Handy for running this workflow against non-default refs for testing.
tsibley
added a commit
that referenced
this pull request
Jun 14, 2024
…rkflow summary A good suggestion by @joverlee521 in review.¹ ¹ <#96 (comment)>
tsibley
force-pushed
the
trs/pathogen-repo-ci/skip-it-skip-it
branch
from
d279c5c
to
89092f6
Compare
|
Rebased to account for later updates made in #95 and resolve conflicts. |
…rkflow summary A good suggestion by @joverlee521 in review.¹ ¹ <#96 (comment)>
tsibley
added a commit
that referenced
this pull request
Jun 14, 2024
…rkflow summary A good suggestion by @joverlee521 in review.¹ ¹ <#96 (comment)>
tsibley
force-pushed
the
trs/pathogen-repo-ci/skip-it-skip-it
branch
2 times, most recently
from
efb5519
to
d46b0d6
Compare
2 tasks
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.
Instead of checking for whether to do anything or not inside each build step individually, move the check to the step's condition. This will make the logs a lot clearer and mean the GitHub workflow metadata stays more in sync with reality.
I've used hashFiles() to check for file existence—it returns the empty string, a falsey value, when there are no matching files—but we could swap that out for using the output of a prior setup step that runs some shell to determine what to run and what to skip. Doing so might make more sense if the conditional becomes more complicated or we want to do more detailed reporting on why steps were skipped or not.
With the changes, the internal-to-this-workflow action, run-nextstrain-ci-build, is no longer that useful and so I've removed it in favor of inlining things. I think this is an improvement for readability.
Related-to: #95 (review)
Checklist