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

Release notes generation #7932

Merged
merged 21 commits into from
Apr 27, 2021
Merged

Conversation

systay
Copy link
Collaborator

@systay systay commented Apr 22, 2021

This PR changes how Vitess creates release-notes for releases.

We'll keep track of which PRs get merged by looking at the git log, and produce the release notes from that.
To group PRs in useful buckets, we'll use labels a bit more than today.

The command looks something like this:

make FROM="upstream/release-10.0" TO="upstream/master" release-notes

This would generate the release notes for all changes that have gone into the master branch since release-10.0 was forked from it.

The produced release notes look something like this:


Bug fixes

Query Serving

CI/Build

Cluster management

Other

vttestserver


First PRs are grouped by type, and then by component. If a PR is missing either, "Other" will be used instead.

Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay marked this pull request as draft April 22, 2021 14:12
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui
Copy link
Member

@systay - 93d1906 introduces the rendering of prPerType using go text templates. A sample output is:

## Other 
### Component: Query Serving
 - Fix bug with reserved connections to stale tablets #7879 
### Other
 - Minor cleanups around errors on the vtgate #7864
 - vreplication: performance & benchmarks (table copying) #7881
 - Planbuilder: Add fuzzer #7902
 - Memory Sort to close the goroutines when callback returns error  #7903
 - Check tablet alias before removing after error stream #7915
## Type: Bug 
### Component: Query Serving
 - Panic when EOF after @ symbol #7925

systay and others added 4 commits April 22, 2021 18:35
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
frouioui and others added 5 commits April 23, 2021 09:06
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@frouioui
Copy link
Member

frouioui commented Apr 23, 2021

Note: we were originally planning on sorting PR information (inside each component) based on their merged date. However, the issue described here cli/cli#3497 occurs. Pull requests are now sorted by their number in increasing order.

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay added Component: Build/CI Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Apr 23, 2021
@systay systay changed the title start release notes tool Release notes generation Apr 23, 2021
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay marked this pull request as ready for review April 23, 2021 14:30
Copy link
Member

@deepthi deepthi 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 awesome!
For anyone else following along, we will use this to generate an initial release notes which will then be edited by hand (curated) to produce a final version.


## Checklist
- [ ] Should this PR be backported?
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 use the Backport me label instead?

Copy link
Member

Choose a reason for hiding this comment

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

Correct. The intention is to get rid of these true/false checklists in pull requests, and instead, use them as Todos.

Copy link
Member

Choose a reason for hiding this comment

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

Then let us add that guidance to the PR template as well.

Copy link
Member

Choose a reason for hiding this comment

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

Added that guidance in c1f6d5b.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And I just removed it. @shlomi-noach raised the valid objection that you need write access to the repo to be able to assign labels. I tweaked the text so it's aimed at new contributors without write access, and not aimed at maintainers.

My suggestion is that we maintainers do it for our own PRs and for any PRs we merge.

Alternatively, we could preface the label section with something that this only applies persons with write access to the repo. Not sure which is the better option ¯\_(ツ)_/¯

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

🎉


If your PR needs to be backported, add the label `Backport me!`.

-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider that only people with Write privileges on our GitHub repo, are able to create labels. Which means the casual contributor is unable to classify their PR by labeling.

I'm not sure if this undermines the purpose of this PR...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it does. We as maintainers should be able to handle labelling before PRs get merged.
And about backporting - deciding what gets backported and what does not has in my experience always been a discussion between maintainers.
Maybe we should just remove this comment altogether from the PR template.

go/tools/release-notes/release_notes.go Outdated Show resolved Hide resolved
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

perhaps add a HTML comment <!-- --> in the PR template to explain how we should go about labeling? Not sure if it's a great idea, because it's aimed for us the maintainers rather than the contributors. At your discretion.

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
@systay systay merged commit 6c82611 into vitessio:master Apr 27, 2021
@systay systay deleted the auto-release-notes branch April 27, 2021 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Build/CI Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants