Skip to content

chore(NODE-5347): add standard release automation #3717

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

Merged
merged 22 commits into from
Jun 21, 2023
Merged

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Jun 8, 2023

Description

What is changing?

Adds CI scripts:

  • pr_list - parses the PRs that are part of a release from the changelog. This filters to fix/feats by nature of using the changelog as the source of truth
  • highlights - uses the new PR template to gather release highlights from each PR listed. It should be robust in that it falls back to returning nothing if it cannot find notes rather than throw an error
  • release_notes - combines highlights and pr_list into a format suitable for the release PR and ensuing github release

Adds CI actions:

  • release_notes - currently a manually dispatched workflow that will run the above scripts, it will change the body of the provided PR number to be the release notes (highlights + changelog entry)
  • release - runs release-please on every push to main, release please handles detecting fix/feats, regenerating the changelog and pr body if so, it uses conventional commits to decide on the next bump (minor/patch). Upon merging the release PR, the steps.release.outputs.release_created condition will be true so npm publish steps will run.
Is there new documentation needed for these changes?

What is the motivation for this change?

  • Automating releases helps us move quicker
  • Publishing from GH allows us to do so with provenance

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken force-pushed the NODE-5347-release branch from bee19ac to 5395b96 Compare June 13, 2023 14:37
@nbbeeken nbbeeken marked this pull request as ready for review June 13, 2023 14:47
@dariakp dariakp self-assigned this Jun 13, 2023
@dariakp dariakp added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jun 13, 2023
@dariakp dariakp self-requested a review June 13, 2023 22:37
name: actions/setup
uses: ./.github/actions/setup
- if: ${{ steps.release.outputs.release_created }}
run: npm publish --provenance
Copy link
Contributor

Choose a reason for hiding this comment

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

do you have a link to your fork or somewhere that tests all this as a dry run?

Copy link
Contributor Author

@nbbeeken nbbeeken Jun 15, 2023

Choose a reason for hiding this comment

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

https://github.com/nbbeeken/node-mongodb-native/releases

Copy link
Contributor Author

@nbbeeken nbbeeken Jun 15, 2023

Choose a reason for hiding this comment

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

So the steps as this PR stands:

  • every releasable unit (fix/feat) commit to main will cause release-please to create/update the release PR
    • It will overwrite the contents of the PR body indiscriminately
  • We currently have to manually run the release_notes action, it gathers the PRs involved in the release and posts the highlights defined in each one to the release PR
    • There's no strong reason to run this any earlier than when we're ready for releasing
    • We can run it early to preview how the highlights look when all gathered together
    • Essentially we use the PRs going into the release as storage for the text that is overwritten by release-please
  • Merging the release PR causes the release, the body of the PR between the horizontal lines: (markdown ---) is used for the body of our release.
    • I think we should have authors go back and edit their PRs & rerun the release_notes action.
    • However, we can make last minute edits to the PR directly and it will be transferred to the GH release.

Copy link
Contributor

Choose a reason for hiding this comment

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

We currently have to manually run the history action, it gathers the PRs involved in the release and posts the highlights defined in each one to the release PR

Why can't we just regenerate the history as a part of generating/updating the release PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Merging the release PR causes the release, the body of the PR between the horizontal lines: (markdown ---) is used for the body of our release.

  • I think we should have authors go back and edit their PRs & rerun the history action.
  • However, we can make last minute edits to the PR directly and it will be transferred to the GH release.

When do PR authors edit their PRs? Is that after we run the release action?

I just want this process to be very clear before we try it on a release with @nbbeeken OOO.


It sounds like there are more steps than just

  • PR author includes highlights in PR description, if necessary
  • PR merges into main, triggering the action that regenerates history, pulls in the new highlights if necessary, and then updates the release notes
  • Release PR is merged for release

What technical limitations are there that prevent the above workflow?

Copy link
Contributor

Choose a reason for hiding this comment

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

also - how do docs factor into the release workflow? Our current practice is to regenerate them and include them in the release commit. Will they be auto-generated as a part of the release and how do we specify that we need to regenerate for a minor or major?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When do PR authors edit their PRs? Is that after we run the release action?

The release_notes workflow collects the highlights from the PRs listed in the changelog, so we can run the workflow on demand after making edits to the relevant PR descriptions.

We don't manually run release that runs on every merge to main and keeps the release PR updated with the latest changelog. When merging the auto generated PR it triggers the actual release.

Your bullets are nearly correct, the generating of our human friendly release notes is manual b/c triggering an action from an action was not easily accomplished. I suspect this to be low hanging fruit we can address in the future.

As I said there's not much gain (beyond previewing) to have this done on each commit, we can think more about this but possibly having it trigger via comments on the release pr like "/regen-notes" to save us the navigation and workflow data entry could be a nice to have.

how do docs factor into the release workflow? Our current practice is to regenerate them and include them in the release commit. Will they be auto-generated as a part of the release and how do we specify that we need to regenerate for a minor or major?

There's no auto docs handling in this work, we will generate them and put them on main after publishing the release. I've captured this in the release instructions.

Copy link
Contributor Author

@nbbeeken nbbeeken Jun 20, 2023

Choose a reason for hiding this comment

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

Why can't we just regenerate the history release_notes as a part of generating/updating the release PR?

I don't have direct access to the PR number & git ref that the auto release is pending at, finding it isn't exactly clear, we'd have to fetch all the PRs and filter, and I'm not sure on what.

Copy link
Contributor

Choose a reason for hiding this comment

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

some updates from slack yesterday

  • release please handles the creating the release PR for whenever non-release PR is merged into the target branch (if the release PR exists, it is updated)
  • release please will run an npm release for us whenever a release PR merges into the target branch
  • release please only gives the PR name / number as output when the release PR is explicitly created. this makes it difficult to update the changelog / release notes in the release PR after a commit lands in main, because we'd have to programmatically detect which PR is the release PR

I think we should include "generating docs into the release PR before release" as the process, instead of generating them after but that doesn't affect the changes in this PR.

@nbbeeken nbbeeken requested a review from dariakp June 15, 2023 20:36
@@ -148,7 +148,6 @@
"fix:eslint": "npm run check:eslint -- --fix",
"prepare": "node etc/prepare.js",
"preview:docs": "ts-node etc/docs/preview.ts",
"release": "bash etc/check-remote.sh && standard-version -a -i HISTORY.md",
Copy link
Contributor

Choose a reason for hiding this comment

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

should we still keep this as an emergency manual release option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can always run this command ourselves, might as well do the clean up here.

release-please can also be run as a CLI tool locally to generate the changelog, so I would reach for that tool first if something in the automation dies

@dariakp dariakp added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Jun 16, 2023
dariakp
dariakp previously approved these changes Jun 20, 2023
name: actions/setup
uses: ./.github/actions/setup
- if: ${{ steps.release.outputs.release_created }}
run: npm publish --provenance
Copy link
Contributor

Choose a reason for hiding this comment

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

We currently have to manually run the history action, it gathers the PRs involved in the release and posts the highlights defined in each one to the release PR

Why can't we just regenerate the history as a part of generating/updating the release PR?

name: actions/setup
uses: ./.github/actions/setup
- if: ${{ steps.release.outputs.release_created }}
run: npm publish --provenance
Copy link
Contributor

Choose a reason for hiding this comment

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

Merging the release PR causes the release, the body of the PR between the horizontal lines: (markdown ---) is used for the body of our release.

  • I think we should have authors go back and edit their PRs & rerun the history action.
  • However, we can make last minute edits to the PR directly and it will be transferred to the GH release.

When do PR authors edit their PRs? Is that after we run the release action?

I just want this process to be very clear before we try it on a release with @nbbeeken OOO.


It sounds like there are more steps than just

  • PR author includes highlights in PR description, if necessary
  • PR merges into main, triggering the action that regenerates history, pulls in the new highlights if necessary, and then updates the release notes
  • Release PR is merged for release

What technical limitations are there that prevent the above workflow?

name: actions/setup
uses: ./.github/actions/setup
- if: ${{ steps.release.outputs.release_created }}
run: npm publish --provenance
Copy link
Contributor

Choose a reason for hiding this comment

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

also - how do docs factor into the release workflow? Our current practice is to regenerate them and include them in the release commit. Will they be auto-generated as a part of the release and how do we specify that we need to regenerate for a minor or major?

@dariakp dariakp merged commit be11dc8 into main Jun 21, 2023
@dariakp dariakp deleted the NODE-5347-release branch June 21, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants