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

Add more links to the changelog and blog #10117

Merged
merged 8 commits into from Dec 31, 2020

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Dec 23, 2020

  • 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?
  • Have you successfully run brew man locally and committed any changes?

We've received some blowback due to the short turn around time between Homebrew 2.6.0 and 2.7.0. 2.7.0 was the release that disabled brew cask commands, so 20 days didn't seem to be quite long enough for the entire community to adapt (namely, some ansible modules weren't updated in time which is causing breakage).

My personal opinion is that 20 days was a tad short for such a major change, but that's irrelevant to this PR (to be clear, not trying to blame any one person for this, it's simply a product of the way we do releases). Regardless of that, I think there are a few places where we could do some work to increase visibility for these releases.

This PR attempts to increase visibility in two ways:

  1. Add a link to the Homebrew Blog to the release notes. It's easy for a user to come across the brew repo and, therefore, the release notes. Only the dedicated users who are really trying will be able to find the blog, though. I think we should add a link to the top of our release notes that links directly to the blog (this already exists in our unused CHANGELOG.md file). For now, I added this to all invocations of brew release-notes. I can, if desired, add an e.g. --major-minor flag to only add this line on major or minor releases (alternatively, there can be some autodetection done on the end_ref if it's passes).
  2. Add links and clearer output to brew update. This PR will now display a message at the very bottom of the brew update output if Homebrew was updated to a new tag. This output will include a link to the release notes. If it's a major or minor release, a link to the blog will be added as well.

@BrewTestBot
Copy link
Member

Review period will end on 2020-12-24 at 17:16:41 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 23, 2020
@reitermarkus
Copy link
Member

short turn around time between Homebrew 2.6.0 and 2.7.0

That's exactly why I think we should deprecate by giving a date instead of on the next “major” version.

@Rylan12
Copy link
Member Author

Rylan12 commented Dec 23, 2020

That's exactly why I think we should deprecate by giving a date instead of on the next “major” version.

@reitermarkus I think you should open a separate issue to address this. I don't want this PR to be a discussion about whether the 2.7.0 release was right/wrong or any of that stuff. I was simply using that as an example to illustrate why I think this is a good idea (which I think is the case regardless of what we do in the future regarding deprecations and whatnot)

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.

Thanks for this @Rylan12, good ideas here!

Library/Homebrew/dev-cmd/release-notes.rb Outdated Show resolved Hide resolved
Library/Homebrew/cmd/update-report.rb Outdated Show resolved Hide resolved
Library/Homebrew/cmd/update-report.rb Show resolved Hide resolved
Library/Homebrew/cmd/update-report.rb Outdated Show resolved Hide resolved
Library/Homebrew/cmd/update-report.rb Outdated Show resolved Hide resolved
@BrewTestBot
Copy link
Member

Review period ended.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 24, 2020
Library/Homebrew/dev-cmd/release-notes.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release-notes.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release-notes.rb Outdated Show resolved Hide resolved
previous_tag_date = Date.parse Utils.popen_read(
"git", "-C", HOMEBREW_REPOSITORY, "log", "-1", "--format=%aI", previous_tag.sub(/\d+$/, "0")
)
opoo "The latest major/minor release was less than one month ago." if previous_tag_date > (Date.today << 1)
Copy link
Member

Choose a reason for hiding this comment

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

What is (Date.today << 1) doing? A variable would also make this more obvious, I think. Date.today - 1.month work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't work:

Error: undefined method `month' for 1:Integer

I'll extract to a variable one_month_ago

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.

Nice work @Rylan12! Please be around to keep an eye for any issues when merging this; it's worth being scared of any changes to the updater code 😁

@Rylan12
Copy link
Member Author

Rylan12 commented Dec 31, 2020

Sounds good. I'm going to merge now. I'll be available for several hours to deal with immediate issues that pop up.

@Rylan12 Rylan12 merged commit e9a44cb into Homebrew:master Dec 31, 2020
@Rylan12 Rylan12 deleted the add-blog-reference-to-release-notes branch December 31, 2020 15:46
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 31, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 31, 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

4 participants