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

docs: update PR template for CHANGELOG reminder #1193

Closed
wants to merge 1 commit into from

Conversation

shaneutt
Copy link
Member

@shaneutt shaneutt commented Jun 8, 2022

What type of PR is this?

/kind documentation

What this PR does / why we need it:

While working on #1183 I found it was quite laborious to search all PR history and fill in the CHANGELOG.md retroactively.

This PR is a proposal instead to indicate that user-facing changes must be accompanied by a CHANGELOG.md update in the PR they are introduced, adding a PR checklist to the PR template. Consequently, this also proposes that we keep a heading of Unreleased in the CHANGELOG as a place for contributors to place their entries prior to release to reduce confusion about where it should go.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2022
@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 8, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shaneutt
To complete the pull request process, please assign dcbw after the PR has been reviewed.
You can assign the PR to them by writing /assign @dcbw in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from jpeach and thockin June 8, 2022 13:40
@shaneutt shaneutt marked this pull request as ready for review June 8, 2022 13:41
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2022
@k8s-ci-robot k8s-ci-robot requested a review from youngnick June 8, 2022 13:41
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @shaneutt! I like this idea. I'm a bit concerned that we may end up with a lot of merge conflicts on CHANGELOG.md if we take this approach, but it could be worth trying to see if that really is an issue.

@@ -14,6 +14,10 @@
- [v0.1.0-rc2](#v010-rc2)
- [v0.1.0-rc1](#v010-rc1)

## Unreleased

Changes that are in `main` but have not yet made it to a release.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add some kind of note here to indicate that unreleased changes may be rolled back or changed prior to a release.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. Change in 1e06e53


**PR Checklist**

- [ ] the `CHANGELOG.md` was updated with any user-facing changes
Copy link
Member

Choose a reason for hiding this comment

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

If we're doing this, we probably want to remove the release-note section above

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Changed in 1e06e53

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 8, 2022
@shaneutt shaneutt requested a review from robscott June 8, 2022 17:46
@shaneutt
Copy link
Member Author

shaneutt commented Jun 9, 2022

@youngnick any feelings on this one?

@youngnick
Copy link
Contributor

youngnick commented Jun 10, 2022

In Contour, we use a changelogs directory, with an unreleased directory below that, and each PR has to include a separate .md file with the changelog in it. This avoids conflicts in a singular changelog file, but makes the release process pretty easy. Ideally, we can make it so your PR won't go green without the changelog present (in Contour we use Actions, but we'd need to figure out a way to add this to Prow's build, I think).

Edit: I think that the current idea is okay for now though.

@shaneutt
Copy link
Member Author

Alright, sounds like so far people are OK with trying this out. I'll bring it up at monday's meeting too, if it's not otherwise merged before then. Thank you.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2022
@shaneutt
Copy link
Member Author

@youngnick, @robscott just wanted to ping on this one because it's been sitting a while: think we're OK to give this one a try?

@robscott
Copy link
Member

Actually before we go too far down this path, maybe we should try the upstream scripting for release notes generation: https://github.com/kubernetes/release/blob/master/cmd/release-notes/README.md.

@shaneutt
Copy link
Member Author

Alright, that goes a bit beyond the scope of what I was trying to accomplish here, so I'll consider this one closed for the time being and then we should probably create a follow up issue for that 👍

@shaneutt shaneutt closed this Jun 29, 2022
@shaneutt shaneutt deleted the patch-2 branch June 29, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants