Skip to content
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

Release workflow #744

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Release workflow #744

wants to merge 6 commits into from

Conversation

acalcutt
Copy link
Contributor

@acalcutt acalcutt commented Jun 29, 2024

Please note, this is untested right now and I am giving it for review and/or ideas for #739.

This is a release workflow for node-pre-gyp. It is based on the node release workflow for maplibre-native at https://github.com/maplibre/maplibre-native/blob/main/.github/workflows/node-release.yml which I worked on getting working.

The idea behind this was, someone needs to approve and add changelog for each release, which should be a PR. When the version in package.json changes, this release workflow will check if that version has been published. If it has not published it, it will do that and also create a github release. the github release will also contain the text of the CHANGELOG.md for the version being published.

The workflow is also made to deal with pre-releases, so if the version is a pre-release, it will publish it as the next npm release

Whoever creates the PR for a new release would follow the instructions in
RELEASE.md

Needs 'NPM_TOKEN' Repository secret. A secret that contains a user npm token with permission to publish

@cclauss
Copy link
Collaborator

cclauss commented Jun 29, 2024

@mapsam Given all the progress that has been made in the past 24 hours, I believe that we need a provisional release of node-pre-gyp to https://www.npmjs.com/package/@mapbox/node-pre-gyp. This pre-release would allow multiple parties to kick the tires (for a week or two?) before we make a general release.

You or one of your colleagues would need to work with @acalcutt to add the required NPM_TOKEN as a repository secret.

If you have a different workflow for solving #739 then we would follow your lead.

@acalcutt
Copy link
Contributor Author

acalcutt commented Jun 29, 2024

Just an FYI, if you did follow my instructions, when running the first command, any 'pre' version will make a pre-release/next release when published. So with a version incremted like
npm version preminor --preid pre --no-git-tag-version
would bring this package to 1.1.0-pre.0 from the current 1.0.11 and the workflow would publish it as a pre-release.

@acalcutt
Copy link
Contributor Author

I gave this a test on my fork, and fixed a few issues and make it more similar to the ci. it should now be working correctly. I used it to make this test release

https://www.npmjs.com/package/@acalcutt/node-pre-gyp/v/2.0.0-pre.1
https://github.com/acalcutt/node-pre-gyp/releases/tag/v2.0.0-pre.1

it was made in this workflow run
https://github.com/acalcutt/node-pre-gyp/actions/runs/9735220985

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

Nice work y'all 🙌 happy to get NPM_TOKEN added as a repo secret for the @mapbox/node-pre-gyp module. I'll report back when that has been added.

Update: see my comment in #739 (comment)

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

@acalcutt @cclauss unsurprisingly, it's a little hard for me to get a static NPM token for our @mapbox/ namespace packages. Internally we're working on a better solution for public repos + npm + outside maintainers that will resolve this. For now I've set a reminder to refresh the token periodically before it is set to expire.

A couple questions:

  • do you need me to modify any of the branch protections for the master branch? Currently no one is allowed to manually push to that branch
  • are there protections to enforce only repo maintainers can trigger a workflow? Just want to be extra cautious outside contributors and forks cannot run the workflow themselves

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

I've added an NPM_TOKEN secret to the repo 👍 I take it back - the tokens I have access to only have 1 hour lifecycles. Working on getting a longer term, static token.

@acalcutt
Copy link
Contributor Author

acalcutt commented Jul 1, 2024

do you need me to modify any of the branch protections for the master branch? Currently no one is allowed to manually push to that branch
This does not require any changes to branch protections.

are there protections to enforce only repo maintainers can trigger a workflow? Just want to be extra cautious outside contributors and forks cannot run the workflow themselves

The only way to trigger this would be to push a version change to the master branch on this repo into package.json. So either by a PR, which a maintainer would have to approve, or by a commit by someone who has admin rights to bypass branch protection.

If someone forks this repo, the workflow will not be able to publish because a valid 'NPM_TOKEN' will not exist in the new repository (it would only exist in this one). They could add their own NPM_TOKEN into their repo, but they would have to have permissions to update the package or it fails.

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

Thanks for the quick response @acalcutt!

@acalcutt
Copy link
Contributor Author

acalcutt commented Jul 1, 2024

For example, to publish my test version, I had to do 3 things

1.) change my package.json package name
https://github.com/acalcutt/node-pre-gyp/blob/master/package.json#L2

2.) change the workflow to check my version of the package is published
https://github.com/acalcutt/node-pre-gyp/blob/master/.github/workflows/release.yml#L26

3.) Provide my own NPM_TOKEN repository secret with my own npm key, that has permission to @acalcutt/node-pre-gyp

@cclauss
Copy link
Collaborator

cclauss commented Jul 2, 2024

I agree that the branch protections should not be changed.

@acalcutt Would you be interested in becoming a maintainer of this repo?

@acalcutt
Copy link
Contributor Author

acalcutt commented Jul 2, 2024

I don't mind submitting PRs once and a while or helping out where I can, but I'm not sure how much of a maintainer I want to be. The things I have submitted so far are mainly stuff I know from trying to keep this limping along for my own projects.

I've mainly submitted these changes in hopes of seeing some up to date releases so I no longer need to maintain @acalcutt/node-pre-gyp. However in my current maplibre-native release I still publish node 16, so to move to this I would have to officially drop it (which i plan to do soon anyway), so using this package would be a breaking change once it is published.

@cclauss
Copy link
Collaborator

cclauss commented Jul 4, 2024

@mapsam Should this be merged or should something else be done in its place?

@mapsam
Copy link
Contributor

mapsam commented Jul 5, 2024

@cclauss if the workflow works for you, it works for me!

My initial questions were just focused on blast-radius security because I wasn't able to get a well-scoped NPM token. I've since been able to generate an NPM_TOKEN only scoped to just the @mapbox/node-pre-gyp module, so my concerns there are settled. That token has been updated in the repo and should be available to test with now 👍

@acalcutt
Copy link
Contributor Author

acalcutt commented Jul 7, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants