consider release-please to improve release process

[gengjiawen]

https://github.com/google-github-actions/release-please-action It's has been using in gyp-next and works pretty great so far as I observed (https://github.com/nodejs/gyp-next/blob/master/.github/workflows/release-please.yml
).

Maybe it's also good here too.

@rvagg What do you think ?

cc @targos @richardlau

[rvagg]

Maybe, I'm not totally opposed to it. What we're doing now doesn't scale very well because it's basically coming down to me managing the releases.

But the install base of node-gyp is very large. 6M weekly downloads. And it's in critical path in CI systems for a very large number of installs. So we have security and stability concerns that should give us pause. And this is all outside of npm where they at least have some stability gating when pulling node-gyp in. So my concern would be the ease with which releases can be made and pushed into people's production pipelines. The write-access on this repo is fairly broad right now because we can gate releases with npm write access.

[targos]

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 only allow a subset of collaborators to publish the package?

[gengjiawen]

The write-access on this repo is fairly broad right now because we can gate releases with npm write access.

Maybe only use it for changelog generate and release PR. remove npm publish step from the workflow.

[rvagg]

Maybe only use it for changelog generate and release PR. remove npm publish step from the workflow.

yes, I was thinking that this might be a good way to get started, gate publish on a human for now and see how we go.

I'd be ok with exploring this switch, it'll help with getting things merged and maintaining a changelog, but we're all going to have to be educated in the commit message pattern (I'm not quite there yet but have been trying to adopt it in other projects). We're clearly not scaling by sticking with the clean merge patterns and metadata of nodejs/node.

[DeeDeeG]

@rvagg here is [read: I wrote] a similar thing to release-please-action... but producing the Node style of CHANGELOG formatting, and working well with Node-style commit prefixes (lib:, doc:, gyp:, and so on) rather than conventional commits prefixes (chore:, fix:, feat:): workflow.yml

It is a GitHub Actions workflow that automatically opens a pull request with an updated CHANGELOG.md (and a bumped version in package.json also).

The PR updates each time master branch is pushed to, if there is anything to add to the CHANGELOG since the last release. It indirectly relies on PR labels (such as semver-major or semver-minor) via changelog-maker, to try to guess if the proposed version bump should be patch, minor, or major. changelog-maker is responsible for very nearly all of the CHANGELOG updating logic. The PR management is done by the peter-evans/create-pull-request GitHub Action.

It's still kind of messy, and the script is unnecessarily long, but that could be improved. (I was hoping to know if this approach is interesting to maintainers here before going all-out polishing it up.)

It works for me, example PR: DeeDeeG#3

P.S.: One shortcoming of this approach (unlike release-please-action) is that it does not git tag the new release and push that tag to GitHub, at the moment. Though that would definitely be doable with GitHub Actions if desired.

P.P.S.: I'm pretty sure I could resolve the remaining issues with #2330/release-please-action and get that approach working instead; now that I'm neck deep in the code here, I can see what was going wrong. Ultimately whatever un-blocks work on this repo is valuable progress in my book.

[rvagg]

One of the problems we have now is that the metadata requirements of the nodejs/node style is too onerous for other people to interact with unless they're experienced in nodejs/node landing or could use git node land to manage it all. The paths ahead I see are:

1. Have more people here who can deal with this process manually (hasn't been working or scaling)
2. Get the git-node tools working for this repo (maybe they already do? maybe that'd be easy?) and then documenting it as a process for landing commits here
3. Switch to an alternative process that doesn't rely on this metadata that people find so difficult. Which is an approach that I thought had some merit since there exists a world of tooling out there surrounding conventional-changelog style and so many people are used to using it so it's not a huge leap if you come contributing here.

[owl-from-hogvarts]

Silly question, what metadata do we rely on now?

[rvagg]

PR-URL: <url to PR> and Reviewed-By: <person>. The PR-URL one is consumed by changelog-maker and the other is a hold-over from requiring at least one review. If you lean into GitHub then you get both of these things from their API, and I think that's probably acceptable for this project.

[DeeDeeG]

I made git-node land usable here with a few minor tweaks. See nodejs/node-core-utils#546.

On the other hand, I'm still working on a clean switch to release-please, so folks here can decide between two working options.

[DeeDeeG]

After playing with it for a bit, I'm clearer on how to deploy release-please.

Technically, all that was needed was using the same markdown header levels for past releases as the ones release-please would produce, so the old entries don't get overwritten/deleted. (DeeDeeG@cd09b65)

I'm working on some documentation on using the new "Conventional Commit" commit prefixes correctly, and other finer details of release-please, mostly for my own understanding to be honest, but could be useful for others as well.

(If wanting to proceed as fast as possible, the commit to fix the Changelog formatting (DeeDeeG@cd09b65), plus #2330, would do it.)

[DeeDeeG]

Documentation/notes about release-please (could be merged as a .md file as documentation for the repo?)

There are three important designations that release-please will recognize:

• bug fixes/patch: Commits starting with fix: or fix(any_nested_prefix): will be recognized as bug fixes, and will trigger a release proposal PR as a patch level bump. They are displayed under the "Bug Fixes" heading in the Changelog, by default.
• features/minor: Commits starting with feat: or feat(any_nested_prefix): will be recognized as introducing new features, and will trigger a release proposal PR as a minor level bump. These are displayed under the "Features" heading in the Changelog, by default.
• breaking/major: Commits starting with any_prefix!: (any prefix ending with !), or with an arbitrary prefix and having a line starting "BREAKING-CHANGE:" in the commit body, will be recognized as introducing breaking changes, and will trigger a release proposal PR as a major version bump. These get a line of text under the "⚠ BREAKING CHANGES" heading in the Changelog, in addition to appearing under the appropriate heading for their prefix.

Caveat about ignored commits:

• Commits with no prefix, or an unrecognized prefix, are ignored entirely by release-please. They will apparently never trigger a release PR, nor will they appear in the Changelog at all. (By default, commits with the chore: prefix are ignored as well.) But this can be overridden in custom configuration if desired.
  • For example, the default commit message when performing a git revert has no prefix:, and will be ignored! Use some appropriate, recognized prefix, like fix: revert so and so, if you want revert commits to show in the Changelog.
  • One could take advantage of this for excluding commits that are not important enough to make the Changelog. For example, start the commit with a prefix no_changelog: do something, or deliberately don't use a prefix.

Customization:
release-please can be taught to recognize arbitrary prefixes. In the Changelog, these are displayed under their custom configured headings. Multiple prefixes can potentially be lumped together under the same heading, for better or worse. (I think grouping multiple prefixes under one heading makes the Changelog less clear at a glance. That said, lots of distinct headings would take up a huge amount of vertical space, making skimming more difficult and requiring more scrolling to read the Changelogs. release-please very mildly discourages bare custom headings in favor of the semver-meaningful fix: and feat:, though prefixes nested inside those are considered semver-meaningful. And changes recognized as semver-major always show in the Changelog.)

Custom prefixes and semver:

• Custom prefixes are all recognized as semver patch level changes, at least on their own.
• To use a custom prefix and have it recognized as semver minor, usefeat(custom_prefix): in the commit message.
  • This will cause it to appear under the "Features" heading in the changelog, with the custom_prefix: in bold before the rest of the commit message.
• To use a custom prefix and have it recognized as semver major, use custom_prefix!:, or use any prefix at all and add BREAKING-CHANGE: some note here about the breaking change as a line in the commit body.
  • Commits with breaking changes will be listed under their configured heading, if the prefix is recognized. A commit with breaking changes and an unrecognized prefix gets a new heading, for example: made_up_prefix!: fix a major issue would show up under the markdown heading "made_up_prefix"
    • Breaking changes commits with exclamation mark prefixes prefix! will have their full commit message appear under both the "⚠ BREAKING CHANGES" heading, and the heading associated with the non-! version of the prefix.
    • Commits with a BREAKING-CHANGES: note about breaking changes here footer will have the note displayed under the "⚠ BREAKING CHANGES" heading, and the first lines of their commit messages displayed under the usual heading for their prefixes. The note and the first line of the commit can be totally different, so the connection may not be obvious to Changelog readers. Therefore, meaningful notes in the BREAKING-CHANGES: footer are important for Changelog readability and utility.

Note: The "⚠ BREAKING CHANGES" heading in the Changelog is unique in that it shows only text, not links to commits. Any changes that provide text under the "⚠ BREAKING CHANGES" heading are listed under another heading as well.

Recommendations with regard to NodeJS-style "topic" commit prefixes:

These prefixes are not guaranteed to be shown in the Changelog, unless specifically configured as supported in the .yml for the release-please-action. Care should be given to including the usual NodeJS-style commit prefixes as supported in the .yml, and/or documentation should advise contributors not to use them without release-please-recognized semver hints, and/or maintainers should have a habit of landing these PRs with manually edited, release-please-friendly commit messages. To take full advantage of automatic semver calculation for the release proposal PRs, and to ensure these commits are included in the Changelog, care should be taken to add semver hints, as appropriate, when landing these. feat(arbitrary_prefix): will indicate semver minor, and be included in the Changelog under "Bug Fixes". arbitrary_prefix! or a BREAKING-CHANGE: footer in the commit message will indicate semver major and be included in the Changelog. For truly patch-level changes, the prefix used in the commit message must be in the release-please-action .yml file, or it will be ignored by/excluded from the release proposal PR. In this case, it is advisable to go into the .yml and add the custom prefix as supported, and/or manually edit the commit back into the Changelog before merging the release proposal PR.