nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

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

Open mmarchini opened 3 years ago

mmarchini commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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[bot] commented 3 years ago

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.

bnb commented 3 years ago

@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 commented 3 years ago

I think @targos set up token in this repo?

targos commented 3 years ago

What token? I don't remember

mmarchini commented 3 years ago

https://github.com/nodejs/node-core-utils/pull/511

targos commented 3 years ago

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