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

chore: auto-publish package to npm after release #511

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

targos
Copy link
Member

@targos targos commented Oct 16, 2020

No description provided.

@targos targos added the do not land PR's that are on hold, and shouldn't land label Oct 16, 2020
@targos
Copy link
Member Author

targos commented Oct 16, 2020

Labeled "do not land" because we need to setup the npm token if this is accepted.

@codecov

This comment has been minimized.

@targos targos requested a review from mmarchini October 16, 2020 13:15
@mmarchini
Copy link
Contributor

This would remove 2FA when we publish. Are we ok with that?

@targos
Copy link
Member Author

targos commented Nov 2, 2020

npm now has automation tokens now, so we don't have to disable 2FA requirements on the package. AFAIU the only risk we would have is that someone with write access to this repo pushes something to steal the token?

@mmarchini
Copy link
Contributor

We still drop 2FA for our "main" publish workflow (assuming the automation becomes the main workflow). It's not ideal IMO but probably low risk enough that we can try it?

@targos
Copy link
Member Author

targos commented Dec 22, 2020

GitHub Actions now have environment protection rules and environment secrets: https://github.blog/changelog/2020-12-15-github-actions-environments-environment-protection-rules-and-environment-secrets-beta/
Maybe that can be used to protect an npm token and/or only allow a subset of collaborators to publish the package?

@targos
Copy link
Member Author

targos commented Mar 17, 2022

I will reimplement the change differently if it's likely to land.

@targos
Copy link
Member Author

targos commented Mar 18, 2022

Updated. We still need to setup an npm automation token before merging this. Who would be able to do that?

@targos targos removed the do not land PR's that are on hold, and shouldn't land label Apr 8, 2022
@targos
Copy link
Member Author

targos commented Apr 8, 2022

The NPM_TOKEN secret is installed. Would you like to review again?

@targos targos merged commit cc2dfa9 into nodejs:main Apr 8, 2022
@targos targos deleted the release-npm branch April 8, 2022 12:34
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.

4 participants