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

[Merged by Bors] - Add "Changelog" and "Migration Guide" to PR template #4143

Closed
wants to merge 9 commits into from

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Mar 7, 2022

Objective

Context: Discord Discussion

Improve the PR template by adding "Changelog" and "Migration Guide" sections. These sections should hopefully help speed up the review/merge process, as well as help make release notes and migration guides.

Solution

Added the "Changelog" section template which suggests listing out the changes of the PR. This also acts as a sort of tl;dr for reviewers (especially for larger PRs).

Added the "Migration Guide" section template which suggests describing how a user might need to migrate their codebase to account for the changes by the PR. This also helps authors/contributors keep the end-user in mind when adding or changing the API.

Both sections are optional— an author does not need to fill these out. Hopefully they will, though, as it provides a handful of really great benefits.

Added the "Changelog" and "Migration Guide" sections for authors to
optionally fill out.
@alice-i-cecile alice-i-cecile added the A-Meta About the project itself label Mar 7, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

IMO Changelog and Migration Guide belong at the top of the template: they're the most important to be able to skim, and the changelog provides very helpful context for reviewer.

.github/pull_request_template.md Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
MrGVSV and others added 2 commits March 7, 2022 15:48
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@MrGVSV
Copy link
Member Author

MrGVSV commented Mar 7, 2022

IMO Changelog and Migration Guide belong at the top of the template: they're the most important to be able to skim, and the changelog provides very helpful context for reviewer.

Should they really go above the objective of the PR? Would this affect how authors view the # Objective section?

@alice-i-cecile
Copy link
Member

Should they really go above the objective of the PR? Would this affect how authors view the # Objective section?

IMO the point of "Objective" is two-fold. First, to link any issues that are closed. Secondly, to help provide stronger context on why the problem is worth solving. These are both valuable, but feel like they're less essential to a casual reader (especially post-merge when browsing changes in release notes) than "here's what changed".

I don't feel super strongly about this though 😅

@MrGVSV
Copy link
Member Author

MrGVSV commented Mar 7, 2022

Another consideration are smaller PRs. If a PR fixes something small like a typo or something similar, do we want to suggest a preference for filling out the template? Such as only filling out the Objective section, or only the Changelog section.

Otherwise we might get:

## Objective 

Fixed typo.

...

## Changelog

### Changed

* Fixed typo.

(Which isn't inherently bad haha just unnecessary)

@MrGVSV
Copy link
Member Author

MrGVSV commented Mar 7, 2022

Should they really go above the objective of the PR? Would this affect how authors view the # Objective section?

IMO the point of "Objective" is two-fold. First, to link any issues that are closed. Secondly, to help provide stronger context on why the problem is worth solving. These are both valuable, but feel like they're less essential to a casual reader (especially post-merge when browsing changes in release notes) than "here's what changed".

I don't feel super strongly about this though 😅

Maybe we add a divider between the two sections and the rest of the template? To further indicate that these are two distinct parts of the PR? And the "real" PR starts in the lower section (objective, solution, etc.)

@alice-i-cecile
Copy link
Member

Another consideration are smaller PRs. If a PR fixes something small like a typo or something similar, do we want to suggest a preference for filling out the template? Such as only filling out the Objective section, or only the Changelog section.

Hmm. I've seen this problem occur even now, where the problem is trivial, as is the solution. Maybe it's Problem and Solution that should be optional?

@MrGVSV
Copy link
Member Author

MrGVSV commented Mar 8, 2022

Another consideration are smaller PRs. If a PR fixes something small like a typo or something similar, do we want to suggest a preference for filling out the template? Such as only filling out the Objective section, or only the Changelog section.

Hmm. I've seen this problem occur even now, where the problem is trivial, as is the solution. Maybe it's Problem and Solution that should be optional?

Well we probably don't want them optional for larger (or even medium-sized) PRs.

@IceSentry
Copy link
Contributor

For the migration guide I think it should be at the bottom. I don't think it helps that much at getting a quick overview.

For the changelog I'm less sure. It's nice to get a quick overview, but a changelog without the full context of the objective isn't necessarily better.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 8, 2022
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
@cart
Copy link
Member

cart commented Mar 16, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 16, 2022
# Objective

Context: [Discord Discussion](https://discord.com/channels/691052431525675048/745355529777315850/950532143325519902)

Improve the PR template by adding "Changelog" and "Migration Guide" sections. These sections should hopefully help speed up the review/merge process, as well as help make release notes and migration guides.

## Solution

Added the "Changelog" section template which suggests listing out the changes of the PR. This also acts as a sort of tl;dr for reviewers (especially for larger PRs).

Added the "Migration Guide" section template which suggests describing how a user might need to migrate their codebase to account for the changes by the PR. This also helps authors/contributors keep the end-user in mind when adding or changing the API.

Both sections are optional— an author does not _need_ to fill these out. Hopefully they will, though, as it provides a handful of really great benefits.

Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
@bors bors bot changed the title Add "Changelog" and "Migration Guide" to PR template [Merged by Bors] - Add "Changelog" and "Migration Guide" to PR template Mar 16, 2022
@bors bors bot closed this Mar 16, 2022
@MrGVSV MrGVSV deleted the pr-template-improvement branch March 16, 2022 23:10
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# Objective

Context: [Discord Discussion](https://discord.com/channels/691052431525675048/745355529777315850/950532143325519902)

Improve the PR template by adding "Changelog" and "Migration Guide" sections. These sections should hopefully help speed up the review/merge process, as well as help make release notes and migration guides.

## Solution

Added the "Changelog" section template which suggests listing out the changes of the PR. This also acts as a sort of tl;dr for reviewers (especially for larger PRs).

Added the "Migration Guide" section template which suggests describing how a user might need to migrate their codebase to account for the changes by the PR. This also helps authors/contributors keep the end-user in mind when adding or changing the API.

Both sections are optional— an author does not _need_ to fill these out. Hopefully they will, though, as it provides a handful of really great benefits.

Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Context: [Discord Discussion](https://discord.com/channels/691052431525675048/745355529777315850/950532143325519902)

Improve the PR template by adding "Changelog" and "Migration Guide" sections. These sections should hopefully help speed up the review/merge process, as well as help make release notes and migration guides.

## Solution

Added the "Changelog" section template which suggests listing out the changes of the PR. This also acts as a sort of tl;dr for reviewers (especially for larger PRs).

Added the "Migration Guide" section template which suggests describing how a user might need to migrate their codebase to account for the changes by the PR. This also helps authors/contributors keep the end-user in mind when adding or changing the API.

Both sections are optional— an author does not _need_ to fill these out. Hopefully they will, though, as it provides a handful of really great benefits.

Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Meta About the project itself S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants