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

Suggestion: auto-land and auto-release NCU Pull Requests #446

Open
mmarchini opened this issue Jun 27, 2020 · 12 comments
Open

Suggestion: auto-land and auto-release NCU Pull Requests #446

mmarchini opened this issue Jun 27, 2020 · 12 comments

Comments

@mmarchini
Copy link
Contributor

Looking at our Pull Requests list, seems like we have six pull requests ready to land (with approvals), but I don't think it's clear who is allowed to land PRs in this repository. Similarly, sometimes commits will sit on master for a while until someone has time to publish a new version on npm. My suggestions:

  1. A scheduled GitHub Action that will merge all PRs with at least one approval and green CI. This could run every 5 minutes (quickest the scheduler can run), or maybe every hour (since we don't have a big flow of PRs here).
  2. A scheduled GitHub Action that will make a new release to npm every day (if we want quick releases) or every week (if we want slower releases)

We would need to identify semverness of PRs somehow, either via labels or using https://www.conventionalcommits.org/en/v1.0.0/. Looking at our logs, we're already not strictly following Node.js core commit guidelines and we have mixed Node.js and conventional commit messages.

As for npm releases, it might be tricky because we require 2FA. If anyone worked with 2FA and automated npm releases before, I'd be happy to see solutions :)

@codebytere
Copy link
Member

codebytere commented Jun 28, 2020

@mmarchini I definitely agree that there's a bit of a bystander effect going on with who has authority to merge things, but i'm not 100% sure that automatic merges are the right solution here for a few reasons:

  1. Fairly few people (i think it's basically ~4-5 of us atm?) maintain this repo, so it's often the case that 1-2 specific people's approval is needed for a given PR as they're the one(s) who'll have the necessary context
  2. CI on this repo is a bit of a struggle bus at present especially on macOS - I see it fail/flake all the time on PRs like this one

What do you think of perhaps adding CODEOWNERS? I think that might make ownership areas a bit more straightforward 🤔

For the latter suggestion, I think weekly releases would be my personal preference but i'd also be ok with daily if that's the consensus from others!

wombat-dressing-room is the best solution to the 2FA problem i can think of off the top of my head - Electron also built a similar thing with https://docs.continuousauth.dev/ but it integrates with Slack so that's probably not ideal for our situation here.

@mmarchini
Copy link
Contributor Author

CODEOWNERS doesn't solve the issue of who can land PRs (the access tab is a mix of individually granted permissions and teams, and it's not clear which team should be pinged to land things here), only who has context to review PRs. It's a great feature to add to the repository, but it wouldn't solve the lingering already-approved PRs issue.

The scope and user base of this project is limited enough that I think daily releases wouldn't be harmful, and fixes would get out faster without needing for a person to start it (unless there's a critical issue which needs to be released right away). I think getting improvements out for collaborators faster would benefit the project. But I wouldn't mind weekly releases either if that's what folks prefer.

The flaky CI is definitely an issue, although if it is a small subset of tests that fail infrequently maybe an automerge would at least offload some PRs?

Thanks for sharing Continuous Auth! I'll definitely take a look.

@bnb
Copy link
Contributor

bnb commented Jul 10, 2020

I've seen that Electron uses Semantic Release for a few of their modules and it seems pretty handy in terms of auto releasing as new features land. Potentially something to consider?

https://github.com/semantic-release/semantic-release

@mmarchini
Copy link
Contributor Author

As a start we could add https://github.com/googleapis/release-please or https://github.com/semantic-release/semantic-release to just create the changelog and release commit automatically (not sure how semantic-release works, release-please will open a PR and will update it every time a commit lands until someone merges it, which is good if we'll publish to npm manually for now). Both follow https://www.conventionalcommits.org/en/v1.0.0/ afaik.

mmarchini added a commit to mmarchini/node-core-utils that referenced this issue Aug 20, 2020
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs#446
mmarchini added a commit to mmarchini/node-core-utils that referenced this issue Aug 20, 2020
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs#446
mmarchini added a commit to mmarchini/node-core-utils that referenced this issue Aug 21, 2020
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs#446
mmarchini added a commit that referenced this issue Aug 21, 2020
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: #446
@mmarchini
Copy link
Contributor Author

Was looking at this again after #488 landed. wombat needs to run on GCP, so we would need donated credits to use it on the Node.js org. CFA doesn't work with Actions yet, which is a blocker for us (I don't think requiring Slack is a problem, we could probably use the Foundation Slack). I'm not aware of any other options. While CFA looks more promising, I'm not sure how it fares on multiple projects with multiple users (assuming we want to enable it to any Node.js project). Would it be possible for a subset of folks to be allowed to release node-core-utils, and a different subset to be allowed to release llnode, for example?

@mmarchini
Copy link
Contributor Author

mmarchini commented Aug 31, 2020

@nodejs/node-core-utils would you be willing to try https://github.com/mmarchini-oss/npm-otp-publish to autopublish when we merge release-please PRs (since I didn't add a README yet, see https://twitter.com/mmarkini/status/1299202760911405060?s=20 for a summary of how it works)? I'm still working on it, but it should be usable as it is today. node-core-utils could be one of the first projects (outside my personal ones) to try it :). The only downside is that we would need an npm bot account and we would need to share the OTP of that account with those of us who want to publish, but it shouldn't be that big a deal IMO.

@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Nov 30, 2020
@mmarchini mmarchini removed the stale label Nov 30, 2020
@bnb
Copy link
Contributor

bnb commented Dec 30, 2020

@mmarchini also worth noting there's now npm automation tokens. Not sure if they'll be a good solution for the specific use case (I wouldn't be surprised if they're not - there's still work going into them to make them more broadly usable), but worth a mention at the very least.

@mmarchini
Copy link
Contributor Author

I think @targos set up token in this repo?

@targos
Copy link
Member

targos commented Jan 5, 2021

What token? I don't remember

@mmarchini
Copy link
Contributor Author

#511

@targos
Copy link
Member

targos commented Jan 7, 2021

Ah, I did not setup the token yet. I don't have access to the bot's npm account.

johnfrench3 pushed a commit to johnfrench3/core-utils-node that referenced this issue Nov 2, 2022
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs/node-core-utils#446
renawolford6 added a commit to renawolford6/node-dev-build-core-utils that referenced this issue Nov 10, 2022
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs/node-core-utils#446
Developerarif2 pushed a commit to Developerarif2/node-core-utils that referenced this issue Jan 27, 2023
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs/node-core-utils#446
gerkai added a commit to gerkai/node-core-utils-project-build that referenced this issue Jan 27, 2023
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs/node-core-utils#446
patrickm68 added a commit to patrickm68/NodeJS-core-utils that referenced this issue Sep 14, 2023
`release-please` will bump the package version and generate changelogs
based on conventionalcommit. It does so by opening a PR every time a
commit lands, and it updates that PR for each subsequent commit.
Publishing to npm is still done manually.

Ref: nodejs/node-core-utils#446
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

No branches or pull requests

4 participants