codebytere commented 4 years ago

This PR constitutes the first portion of an automated release flow to abstract away the tedious aspects of preparing and promoting releases.

The preparation and promotion aspects of this new flow will be split into two separate PRs for easier review, so this is just the preparation step, with the promotion step marked as a TODO for implementation in a subsequent PR.

There are several things i'd like to refactor to make this a little less verbose in the future, but to avoid scope creep for now i've just marked some of them TODO and unless you all feel they are blocking i'll move to address them in follow-ups.

codecov[bot] commented 4 years ago

Codecov Report

Merging #388 into master will increase coverage by 0.29%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #388      +/-   ##
==========================================
+ Coverage   75.98%   76.28%   +0.29%     
==========================================
  Files          21       21              
  Lines        1445     1480      +35     
==========================================
+ Hits         1098     1129      +31     
- Misses        347      351       +4

Impacted Files	Coverage Δ
lib/pr_checker.js	`96.13% <0.00%> (-1.34%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 66037a6...be4c952. Read the comment docs.

Trott commented 4 years ago

@nodejs/releasers

targos commented 4 years ago

Thanks for working on this!

MylesBorins commented 4 years ago

Just tried to use it and got an error

⠋ Fetching branch-diffTypeError: Cannot read property 'onlyNotableChanges' of undefined
    at ReleasePreparation.getBranchDiff (/Users/mborins/code/node-core-utils/lib/prepare_release.js:352:14)
    at main (/Users/mborins/code/node-core-utils/components/git/release.js:84:22)
    at release (/Users/mborins/code/node-core-utils/components/git/release.js:59:21)
    at Object.handler (/Users/mborins/code/node-core-utils/components/git/release.js:42:16)
    at Object.runCommand (/Users/mborins/code/node-core-utils/node_modules/yargs/lib/command.js:240:40)
    at Object.parseArgs [as _parseArgs] (/Users/mborins/code/node-core-utils/node_modules/yargs/yargs.js:1145:41)
    at Object.get [as argv] (/Users/mborins/code/node-core-utils/node_modules/yargs/yargs.js:1079:21)
    at Object.<anonymous> (/Users/mborins/code/node-core-utils/bin/git-node:20:3)
    at Module._compile (internal/modules/cjs/loader.js:1147:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1167:10)

MylesBorins commented 4 years ago

Also... for git node release --prepare it would be nice to have a default that determined for you the type of release (patch / minor / major) rather than having to provide the explicit version number.

MylesBorins commented 4 years ago

Some bugs I found playing with this

I provided the version as v13.11.0 and the branch it crearted was vv13.11.0 (and similar duplication across meta data. Should likely not do that 😅 (ignore v)
I accidentally said yes to changing module version number which resulted in reformatting the entire module version data and thus a lot of churn. Might want to make sure that generated module version number information has a consistent style. Honestly the question came as a surprise... and we should maybe only ask it for Semver-Major release (imho)

edit:

was also missing a newline between the new changelog and old changlog (after the list of commits)

MylesBorins commented 4 years ago

I also just accidentally clicked through on the "create PR" before I had a chance to audit stuff... would prefer to not have that step 😅

edit:

opening the PR failed lol

Error: remote:
remote: Create a pull request for 'v13.11.0-proposal' on GitHub by visiting:
remote:      https://github.com/nodejs/node/pull/new/v13.11.0-proposal
remote:
To github.com:nodejs/node.git
 * [new branch]            v13.11.0-proposal -> v13.11.0-proposal

    at exports.runSync (/Users/mborins/code/node-core-utils/lib/run.js:60:11)
    at ReleasePreparation.openPullRequest (/Users/mborins/code/node-core-utils/lib/prepare_release.js:175:20)
    at ReleasePreparation.prepare (/Users/mborins/code/node-core-utils/lib/prepare_release.js:118:10)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

MylesBorins commented 4 years ago

One other bit... while it is nice having the notable commit information in the commit message we likely don't want it as markdown

edit:

also noticed that my username is all in lower caps in the changelog

MylesBorins commented 4 years ago

Automation is missing one of the link references at the top of the version specific changelog

https://github.com/nodejs/node/pull/32218

MylesBorins commented 4 years ago

dogfooded again. Everything worked great until I attempted to push upstream and then it bailed with the following warning

Error: remote:
remote: Create a pull request for 'v13.12.0-proposal' on GitHub by visiting:
remote:      https://github.com/nodejs/node/pull/new/v13.12.0-proposal
remote:
To github.com:nodejs/node.git
 * [new branch]            v13.12.0-proposal -> v13.12.0-proposal

    at exports.runSync (/Users/mborins/code/node-core-utils/lib/run.js:60:11)
    at ReleasePreparation.pushBranch (/Users/mborins/code/node-core-utils/lib/prepare_release.js:247:5)
    at ReleasePreparation.prepare (/Users/mborins/code/node-core-utils/lib/prepare_release.js:160:10)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

rvagg commented 4 years ago

wow, this is a great way to deal with release complexity, and it even edits abi_version_registry!

Minor suggestion is to add changelog-maker to the dependencies list, not strictly essential but since you're directly relying on the executable it it may be a good idea to lock the version to the code rather than whatever branch-diff pulls in.

MylesBorins commented 4 years ago

Seems like the href for the prior release is getting replaced rather than appended in CHANGELOG_vX.x

https://github.com/nodejs/node/pull/32376#discussion_r395615448

MylesBorins commented 4 years ago

trying again and the "auto detect" wasn't working for me... it kept trying to make a v14.x release and complaining that i was on v13.x (for a 13.x release)

targos commented 4 years ago

Rubberstamp LGTM. I'll try it for the next release I prepare.

nodejs / node-core-utils

git-node: add git-node-release #388

Codecov Report