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.

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

Use GitHub API to generate release notes #12196

Merged
merged 1 commit into from Oct 7, 2021

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Oct 5, 2021

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This PR removes the ReleaseNotes module in favor of using GitHub's new release notes generation API for the brew release command.

Now, brew release will use the API to generate the release notes since the previous tag. Because of the release configuration we have for this repo, PRs opened by @dependabot or @BrewTestBot are already ignored. The contents returned by the API call are used directly as the body of the draft release that will be created.

Additionally, if brew release is run with --major or --minor, a second API call will be made to generate the release notes since the most recent major/minor release. These are then processed to remove the new contributor section, remove titles and blank lines, remove the changelog link, remove the GitHub handles, and format the items in a convenient markdown format to be easily copied into a blog post (this format is the same as existed before this PR).

@BrewTestBot
Copy link
Member

Review period will end on 2021-10-06 at 18:56:11 UTC.

@BrewTestBot BrewTestBot added waiting for feedback Merging is blocked until sufficient time has passed for review and removed waiting for feedback Merging is blocked until sufficient time has passed for review labels Oct 5, 2021
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

This is wonderful ❤️

.sort
# release notes without usernames, new contributors, or extra lines
blog_post_notes = GitHub.generate_release_notes("Homebrew", "brew", new_version,
previous_tag: latest_major_minor_version)["body"]
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to handle if ["body"] doesn't exist? Alternatively, shall we just return that from the generate_release_notes method so that nil or "" can be better handled here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's probably more consistent with the other GitHub API methods if we don't return nil (although it would make handling it here easier). When I have some more time (probably this weekend) I'll play with it a bit more to see what's returned if the request fails to better handle those cases

Copy link
Member

Choose a reason for hiding this comment

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

@Rylan12 Fine with me either way!

@MikeMcQuaid MikeMcQuaid added critical Critical change which should be shipped as soon as possible. and removed critical Critical change which should be shipped as soon as possible. labels Oct 7, 2021
@BrewTestBot
Copy link
Member

Review period ended.

@MikeMcQuaid MikeMcQuaid merged commit 96e0600 into Homebrew:master Oct 7, 2021
@MikeMcQuaid
Copy link
Member

Thanks again @Rylan12! Think this is good to merge as-is and can iterate on it this weekend if desired.

@Rylan12 Rylan12 deleted the release-notes-api branch October 9, 2021 17:03
@Rylan12
Copy link
Member Author

Rylan12 commented Oct 9, 2021

I looked into this a bit more. If the API request fails, it looks like a GitHub::API::Error exception is raised, so the ["body"] is never reached. The most likely error is probably a PAT scope issue which would look like this:

$ brew release
==> Creating draft release for version 3.2.16
Error: Your HOMEBREW_GITHUB_API_TOKEN credentials do not have sufficient scope!
Scopes required: repo
Scopes present:  none
Create a GitHub personal access token:
    https://github.com/settings/tokens/new?scopes=gist,public_repo,workflow&description=Homebrew
  echo 'export HOMEBREW_GITHUB_API_TOKEN=your_token_here' >> ~/.zshrc

Error: Not Found

I think that's pretty clear so I don't feel the need to improve the messaging.

There's a chance that the API fails when there isn't an API issue that might look something like this (created by requesting release notes from a non-existent tag):

$ brew release
==> Creating draft release for version 3.2.16
Error: Invalid previous_tag parameter

This is probably pretty unlikely to occur, but if it does it's certainly not very clear what's happening. Since it's not likely, I feel okay leaving it as is and assuming that whoever is running the command is a Homebrew/brew maintainer and therefore is comfortable sorting through the error on their own. Alternatively, we could modify the error message to say e.g. GitHub API Error: Invalid previous_tag parameter or something like that.

I won't plan on opening a PR, though, unless you think it's worth improving further.

@MikeMcQuaid
Copy link
Member

I think that's pretty clear so I don't feel the need to improve the messaging.

This is probably pretty unlikely to occur, but if it does it's certainly not very clear what's happening. Since it's not likely, I feel okay leaving it as is and assuming that whoever is running the command is a Homebrew/brew maintainer and therefore is comfortable sorting through the error on their own.

I won't plan on opening a PR, though, unless you think it's worth improving further.

All 👍🏻, thanks for investigating @Rylan12!

@github-actions github-actions bot added the outdated PR was locked due to age label Nov 11, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants