chore(NODE-6243): move Node release tooling to drivers-github-tools #45
Conversation
baileympearson
changed the title
node/release_template.yml
Outdated
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Node and dependencies | ||
| uses: baileympearson/drivers-github-tools/node/setup@add-signing-env-action-for-node |
There was a problem hiding this comment.
Just a reminder to adjust this before merging this PR
nbbeeken
added
the
Primary Review
| shell: bash | ||
| run: | | ||
| package_version=$(jq --raw-output '.version' package.json) | ||
| echo "package_version=${package_version}" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Why did this change to ENV vs OUTPUT?
There was a problem hiding this comment.
ENV is easier to work with because anything can be accessed with env.<NAME> and doesn't need to worry about step ids.
There was a problem hiding this comment.
It may be easier to use but it removes the explicit linkages of dependence between steps, the need to reference ids is a feature not a burden
There was a problem hiding this comment.
Note that as per the Workflow syntax for GitHub Actions, steps don't have outputs. The syntax steps.<step-id>.outputs.<output-id> is used to access outputs from an action called in that particular step. The $GITHUB_OUTPUT env variable is shared between all steps in a job, so it's no different from $GITHUB_ENV, with the subtle difference that $GITHUB_ENV is not made available as a job output.
There was a problem hiding this comment.
steps don't have outputs.
Sorry a bit confused here, each step can have outputs, the steps context is how you access them. Here's an example of how we use them in the driver
used to access outputs from an action called in that particular step
When you've called away to an action, it can return specified values by declaring its outputs: https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#outputs-for-composite-actions
Whether you're calling away to an external action or running an inline script step using outputs is an alternative to using env. The benefit of outputs is the requirement to declaratively connect your ins and outs.
There was a problem hiding this comment.
[...] the
stepscontext is how you access them
Welp, of course it'd be in the step context, not in the workflow syntax. I stand corrected - I assumed the outputs were only available when calling to a separate action in a step.
node/sign_js_only_package/action.yml
Outdated
| dry_run: | ||
| description: 'Should we upload files to the release?' | ||
| required: false | ||
| default: 'true' |
There was a problem hiding this comment.
Is it intentional that you have to set dry_run to false to get this to work?
There was a problem hiding this comment.
Yes, it seemed safer to default to not uploading to avoid accidental uploads while modifying the scripts. I think that's a sensible default to leave going forward too.
There was a problem hiding this comment.
Won't scripts already have dry_run set to false because they needed to do so? Where does the safety come in once the false settings are out there?
dry run is opt-in with our other tools, it is surprising that it is reversed here
baileympearson
force-pushed
the
add-signing-env-action-for-node
branch
from
8a279b8 to
b1e4e6f
Compare
|
@baileympearson I leave merging to you after you've updated the action names |
Moves the signing action and the setup action to drivers-github-tools. This also creates a release action template and a generation script for js-only packages, to simplify generating release actions for each repo and to ensure that they're the same across all our repos.
It turns out that sharing workflows requires workflow files to be put in the .github/workflows folder. So unless we want to have node-specific release actions in the .github/workflows folder in drivers-github-tools, we can't re-use workflows as a whole. I chose not to shared workflows and instead generate release files (see the generation script in drivers-github-tools).
This tooling is working for the both native packages and non-native packages:
driver: mongodb/node-mongodb-native#4159
kerberos: mongodb-js/kerberos#179
Once this PR merges, I'll adjust PRs against the driver, bson and legacy driver main and open them for review.