forked from kousu/spinalcordtoolbox-binaries
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update GHA workflow structure to match other repos (workflow-dispatch
, NodeJS 12 actions, etc.)
#6
Merged
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
According to these changelogs: https://github.com/actions/upload-artifact/releases/tag/v3.0.0 https://github.com/actions/upload-artifact/releases/tag/v3.1.1 This should address the Github Actions warnings about upgrading from node12 to node16, and about not using `set-output`.
This involves a change of behavior: https://github.com/actions/download-artifact#compatibility-between-v1-and-v2v3 Namely, artifacts are now downloaded to the current directory by default, instead of creating a (redundant, in our case) directory with the artifact name.
The README for actions/create-release mentions that it's currently unmaintained, and points to some alternatives, including ncipollo/release-action, which we use in other projects already. The API is different, so this means changing how the parameters are specified for the action, but it seems like a fairly straightforward match. The bonus is that ncipollo/release-action allows attaching artifacts directly, which means: * The three steps with actions/download-artifact have to happen earlier; * The three steps with actions/upload-release-asset (which is also an archived, unmaintained action) don't need to happen at all; and * No other steps refer to the `create_release` step id, so it can be removed.
mguaypaq
commented
Nov 8, 2023
It looks like before this commit, we would automatically run the workflow and create the release for _any_ changes whatsoever. Then, we would pick one of those automatically-created releases and rename it to the desired release title. (Perhaps this workflow was first developed before `workflow-dispatch` even existed?) In other repos, though, we follow a "don't automatically create releases, only create releases when manually run" sort of workflow. And, given that `sct_tutorial_data` follows a similar pattern (generate artifacts, upload them to a release), I figure we can follow its example here. Plus, by manually specifying the release title, we make sure to avoid any conflicts, eliminating the need for the run ID.
This mimics `sct_tutorial_data` et al.
Since we're no longer creating all these intermediate releases, and only creating a release when manually choosing to, I think it makes more sense to just create a draft release (rather than a non-draft prerelease). This also matches what we do for `sct_tutorial_data`.
joshuacwnewton
changed the title
Upgrade node12 actions
Update GHA workflow structure to match other repos (Nov 9, 2023
workflow-dispatch
, NodeJS 12 actions, etc.)
mguaypaq
commented
Nov 10, 2023
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.
I can't click the Approve button because I was the original author of the PR, but it looks good! Thanks for the bigger overhaul.
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.
This PR updates the structure of the workflow file to match the conventions we've developed in other repos. Namely:
workflow-dispatch
to manually trigger the releaseif:
condition so that, for PRs, every step is run but the actual release stepactions/
withncipollo/
)Original PR description (Upgrading outdated NodeJS 12 actions)
I made several trivial upgrades of node12 actions that Github Actions warns about today, but this one is more involved. I'm not sure how to test it, so maybe this should stay as a draft PR until we're ready to create a new release of the SCT binaries?
Although, since this also fixes a permissions bug, maybe we could make a release?
This should address the following warnings from Github Actions:
It also replaces the archived, unmaintained actions
actions/create-release
andactions/upload-release-asset
withncipollo/release-action
, which is maintained and which we use for other repos already.