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.

Sign up for GitHub

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

Jump to bottom

meta: add release-please-action for automated releases #2395

Merged
merged 5 commits into from May 19, 2021
Merged

meta: add release-please-action for automated releases #2395

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd09b65
doc: update CHANGELOG markdown for release-please
DeeDeeG May 11, 2021
177c0f0
ci: add .github/workflows/release-please.yml
DeeDeeG May 8, 2021
92ae1c1
chore: update commit guidelines in PR template
DeeDeeG May 11, 2021
c47b6f2
ci: remove "travis" prefix for release-please
DeeDeeG May 12, 2021
4df4098
ci: add more prefixes for release-please
DeeDeeG May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ Contributor guide: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
- [ ] `npm install && npm test` passes
- [ ] tests are included <!-- Bug fixes and new features should include tests -->
- [ ] documentation is changed or added
- [ ] commit message follows [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)
- [ ] commit message follows [commit guidelines](https://github.com/googleapis/release-please#how-should-i-write-my-commits)
Copy link
Contributor Author

@DeeDeeG DeeDeeG May 11, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could link to Electron's commit message guidelines, which are a lot like Node's (forked docs based on NodeJS's docs), but based around Conventional Commits: https://github.com/electron/electron/blob/master/docs/development/pull-requests.md#commit-message-guidelines

On the other hand, emphasizing fix: feat: and BREAKING-CHANGE: semver hints would help for the release proposal PRs to know whether to bump patch, minor or major. Maybe we should have our own docs page about this?

For dealing with semver manually, release-please-action can be told to release a specific version, by posting a commit with a Release-As: x.y.z footer: https://github.com/googleapis/release-please/tree/v11.11.0#how-do-i-change-the-version-number. So maintainers can just do that and not worry very much about individual commit messages, if the need arises.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link to release-please.yml.


##### Description of change
<!-- Provide a description of the change -->
Expand Down
39 changes: 39 additions & 0 deletions .github/workflows/release-please.yml
View file
Open in desktop
@@ -0,0 +1,39 @@
name: release-please

on:
push:
branches:
- master
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

master or main ?!?


jobs:
release-please:
runs-on: ubuntu-latest
steps:
- uses: google-github-actions/release-please-action@v2
id: release
with:
package-name: node-gyp
release-type: node
changelog-types: >
[{"type":"feat","section":"Features","hidden":false},
{"type":"fix","section":"Bug Fixes","hidden":false},
{"type":"bin","section":"Core","hidden":false},
{"type":"gyp","section":"Core","hidden":false},
{"type":"lib","section":"Core","hidden":false},
{"type":"src","section":"Core","hidden":false},
{"type":"test","section":"Tests","hidden":false},
{"type":"build","section":"Core","hidden":false},
{"type":"configure","section":"Core","hidden":false},
{"type":"install","section":"Core","hidden":false},
{"type":"deps","section":"Core","hidden":false},
{"type":"python","section":"Core","hidden":false},
{"type":"win","section":"Core","hidden":false},
{"type":"zos","section":"Core","hidden":false},
{"type":"doc","section":"Doc","hidden":false},
{"type":"docs","section":"Doc","hidden":false},
{"type":"readme","section":"Doc","hidden":false},
{"type":"chore","section":"Miscellaneous","hidden":false},
{"type":"refactor","section":"Miscellaneous","hidden":false},
{"type":"ci","section":"Miscellaneous","hidden":false},
{"type":"travis","section":"Miscellaneous","hidden":false},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis looks redundant here

Copy link
Contributor Author

@DeeDeeG DeeDeeG May 12, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback.

Travis makes sense to remove, especially since we are only using GitHub Actions now. It's not hurting anything, I think, but it is odd to see it there.

This list is the full set of prefixes that will be included in (read: won't be omitted from) the Changelog.

I expect people won't use most of these, but commit prefixes that release-please doesn't recognize are left out of the Changelog entirely. I added almost every prefix I saw in the commit history, going all the way back to the first commit to just before the early releases, although I ignored most of the stuff from before NodeJS-style commit metadata were in use. My thinking was "the more the better," for comprehensive coverage. I didn't want anything that is merged to be left out of the Changelog.

On the other hand, leaving certain prefixes out of the list would be a way to intentionally exclude them from the final Changelog. If that would be preferred, (to "reduce clutter" in the Changelog, I suppose?), then I can remove some more of these. (For example: the default for chore: is exclusion from the Changelog, but now with chore: on the list, those commits will be included. I thought including them was better.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ones I missed or decided not to include:

  • tools: (used for some small gyp tweaks back in 2018/2019: ea9aff4, e8ea74e, 6f5a408. Two of these patches were ported from nodejs/node's tools/gyp. the original prefix from nodejs/node was preserved.)
  • History: (apparently CHANGELOG.md used to be History.md. 89fe781)
  • addon: (another small gyp tweak: be4ea59)
  • rebuild: (adjustments to the node-gyp rebuild subcommand: 2f675ab, 0eacc90)
  • package: (Various updates to package.json, e.g.: d535620. This doesn't seem like a good prefix to use, in my opinion.)
  • nopt: (Widening the version range for the nopt package in package.json... a93752f.)
  • node-gyp: (Updates to the lib/node-gyp.js file: e82e438, b4302a3, c4bb689)
  • xcode_emulation: (A patch cherry-picked from upstream gyp that allows running on macOS with "command line tools" rather than full Xcode. The original prefix from upstream was preserved: 5785bae)
  • node_modules: (pruning unnecessary dirs from the (at that time checked-in) node_modules dir: 484b286)
  • addon.gypi: (adjustments to the addon.gypi file)
  • remove (the remove subcommand of node-gyp (lib/remove.js): 800179a, 119e030)
  • legacy: (an old legacy dir, now removed.)
  • jshintrc: (An old .jshintrc config file, now removed: b3a147e)
  • windows: (fully-spelled-out win:: 23bd601)
  • list: (The list subcommand of node-gyp (lib/list.js): ae4d8d1)
  • core: (Various updates to the files under lib/: a185536, ae39e1a, 62e5851, 1354d27)
  • 'clean': (adjustment to the clean subcommand of node-gyp... quoted for some reason: 06aac83)

Of these, I think I want to add:

  • The rest of the subcommands (clean:, list:, rebuild:, remove:).
    • And to complete the lib/ folder set, maybe node-gyp: for lib/node-gyp.js, but I think we should really use lib: for that instead.
  • Probably add windows:, which in turn makes me want to add mac: and macOS:, linux: and maybe lin:.

Copy link
Contributor Author

@DeeDeeG DeeDeeG May 12, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Although I think maybe encouraging people to just use chore: fix: and feat: might be simpler and for the best. In which case dropping all the custom configured prefixes might be the most appropriate move. (And consider adding a bot that enforces Conventional Commits commit messages? Which might annoy/drive away new contributors... maintainer discipline in what messages are used when landing commits would also take care of the problem without the need for a bot.)

Edit to add: It is possible to lint the commit messages to conform with Conventional Commits with a GitHub Action. I've seen this one used before: https://github.com/marketplace/actions/commit-linter

{"type":"meta","section":"Miscellaneous","hidden":false}]