-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Release notes generation #7932
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
Changes from 18 commits
7acc336
93d1906
9bfb1ea
893fb04
5d5db85
2e20f61
77ad62c
15fc3b5
655c234
1154b46
b2f2f35
f002659
ca934c3
797bfa6
e933be3
d2b4a3b
c1f6d5b
8126436
aa95ea4
8c8393e
28a0f09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,23 +10,21 @@ | |
| <!-- A few sentences describing the overall goals of the pull request's commits. --> | ||
|
|
||
| ## Related Issue(s) | ||
| <!-- List related issues and pull requests: --> | ||
| <!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #???? --> | ||
|
|
||
| - | ||
|
|
||
| ## Checklist | ||
| - [ ] Should this PR be backported? | ||
| - [ ] Tests were added or are not required | ||
| - [ ] Documentation was added or is not required | ||
|
|
||
| ## Deployment Notes | ||
| <!-- Notes regarding deployment of the contained body of work. These should note any db migrations, etc. --> | ||
|
|
||
| ## Impacted Areas in Vitess | ||
| Components that this PR will affect: | ||
| <!-- Labeling PRs | ||
|
|
||
| - [ ] Query Serving | ||
| - [ ] VReplication | ||
| - [ ] Cluster Management | ||
| - [ ] Build/CI | ||
| - [ ] VTAdmin | ||
| For each PR, try to find one `Type:` label and one `Component:` label. | ||
| This will help in finding a reviewer, and in creating release notes for the next release. | ||
|
|
||
| If your PR needs to be backported, add the label `Backport me!`. | ||
|
|
||
| --> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider that only people with I'm not sure if this undermines the purpose of this PR...
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,265 @@ | ||
| /* | ||
| Copyright 2021 The Vitess Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package main | ||
|
systay marked this conversation as resolved.
|
||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "flag" | ||
| "fmt" | ||
| "log" | ||
| "os" | ||
| "os/exec" | ||
| "regexp" | ||
| "sort" | ||
| "strings" | ||
| "sync" | ||
| "text/template" | ||
| ) | ||
|
|
||
| type ( | ||
| label struct { | ||
| Name string `json:"name"` | ||
| } | ||
|
|
||
| prInfo struct { | ||
| Labels []label `json:"labels"` | ||
| Number int `json:"number"` | ||
| Title string `json:"title"` | ||
| } | ||
|
|
||
| prsByComponent = map[string][]prInfo | ||
|
|
||
| prsByType = map[string]prsByComponent | ||
|
|
||
| sortedPRComponent struct { | ||
| Name string | ||
| PrInfos []prInfo | ||
| } | ||
|
|
||
| sortedPRType struct { | ||
| Name string | ||
| Components []sortedPRComponent | ||
| } | ||
| ) | ||
|
|
||
| const ( | ||
| markdownTemplate = ` | ||
| {{- range $type := . }} | ||
| ## {{ $type.Name }} | ||
| {{- range $component := $type.Components }} | ||
| ### {{ $component.Name }} | ||
| {{- range $prInfo := $component.PrInfos }} | ||
| - {{ $prInfo.Title }} #{{ $prInfo.Number }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| ` | ||
|
|
||
| prefixType = "Type: " | ||
| prefixComponent = "Component: " | ||
| ) | ||
|
|
||
| func loadMergedPRs(from, to string) ([]string, error) { | ||
| cmd := exec.Command("git", "log", "--oneline", fmt.Sprintf("%s..%s", from, to)) | ||
| out, err := cmd.Output() | ||
| if err != nil { | ||
| execErr := err.(*exec.ExitError) | ||
| return nil, fmt.Errorf("%s:\nstderr: %s\nstdout: %s", err.Error(), execErr.Stderr, out) | ||
| } | ||
|
|
||
| var prs []string | ||
| rgx := regexp.MustCompile(`Merge pull request #(\d+)`) | ||
| lines := strings.Split(string(out), "\n") | ||
| for _, line := range lines { | ||
| lineInfo := rgx.FindStringSubmatch(line) | ||
| if len(lineInfo) == 2 { | ||
| prs = append(prs, lineInfo[1]) | ||
| } | ||
| } | ||
|
|
||
| sort.Strings(prs) | ||
| return prs, nil | ||
| } | ||
|
|
||
| func loadPRinfo(pr string) (prInfo, error) { | ||
| cmd := exec.Command("gh", "pr", "view", pr, "--json", "title,number,labels") | ||
| out, err := cmd.Output() | ||
| if err != nil { | ||
| execErr := err.(*exec.ExitError) | ||
| return prInfo{}, fmt.Errorf("%s:\nstderr: %s\nstdout: %s", err.Error(), execErr.Stderr, out) | ||
| } | ||
| var prInfo prInfo | ||
| err = json.Unmarshal(out, &prInfo) | ||
| return prInfo, err | ||
| } | ||
|
|
||
| func loadAllPRs(prs []string) ([]prInfo, error) { | ||
| errChan := make(chan error) | ||
| wgDone := make(chan bool) | ||
| prChan := make(chan string, len(prs)) | ||
| // fill the work queue | ||
| for _, s := range prs { | ||
| prChan <- s | ||
| } | ||
| close(prChan) | ||
|
|
||
| var prInfos []prInfo | ||
| fmt.Printf("Found %d merged PRs. Loading PR info", len(prs)) | ||
| wg := sync.WaitGroup{} | ||
| mu := sync.Mutex{} | ||
| for i := 0; i < 10; i++ { | ||
| wg.Add(1) | ||
| go func() { | ||
| // load meta data about PRs | ||
| defer wg.Done() | ||
| for b := range prChan { | ||
| fmt.Print(".") | ||
| prInfo, err := loadPRinfo(b) | ||
| if err != nil { | ||
| errChan <- err | ||
| break | ||
| } | ||
| mu.Lock() | ||
| prInfos = append(prInfos, prInfo) | ||
| mu.Unlock() | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| go func() { | ||
| // wait for the loading to finish | ||
| wg.Wait() | ||
| close(wgDone) | ||
| }() | ||
|
|
||
| var err error | ||
| select { | ||
| case <-wgDone: | ||
| break | ||
| case err = <-errChan: | ||
| break | ||
| } | ||
|
|
||
| fmt.Println() | ||
| return prInfos, err | ||
| } | ||
|
|
||
| func groupPRs(prInfos []prInfo) prsByType { | ||
| prPerType := prsByType{} | ||
|
|
||
| for _, info := range prInfos { | ||
| var typ, component string | ||
| for _, lbl := range info.Labels { | ||
| switch { | ||
| case strings.HasPrefix(lbl.Name, prefixType): | ||
| typ = strings.TrimPrefix(lbl.Name, prefixType) | ||
| case strings.HasPrefix(lbl.Name, prefixComponent): | ||
| component = strings.TrimPrefix(lbl.Name, prefixComponent) | ||
| } | ||
| } | ||
| switch typ { | ||
| case "": | ||
| typ = "Other" | ||
| case "Bug": | ||
| typ = "Bug fixes" | ||
| } | ||
|
|
||
| if component == "" { | ||
| component = "Other" | ||
| } | ||
| components, exists := prPerType[typ] | ||
| if !exists { | ||
| components = prsByComponent{} | ||
| prPerType[typ] = components | ||
| } | ||
|
|
||
| prsPerComponentAndType := components[component] | ||
| components[component] = append(prsPerComponentAndType, info) | ||
| } | ||
| return prPerType | ||
| } | ||
|
|
||
| func createSortedPrTypeSlice(prPerType prsByType) []sortedPRType { | ||
| var data []sortedPRType | ||
| for typeKey, typeElem := range prPerType { | ||
| newPrType := sortedPRType{ | ||
| Name: typeKey, | ||
| } | ||
| for componentKey, prInfos := range typeElem { | ||
| newComponent := sortedPRComponent{ | ||
| Name: componentKey, | ||
| PrInfos: prInfos, | ||
| } | ||
| sort.Slice(newComponent.PrInfos, func(i, j int) bool { | ||
| return newComponent.PrInfos[i].Number < newComponent.PrInfos[j].Number | ||
| }) | ||
| newPrType.Components = append(newPrType.Components, newComponent) | ||
| } | ||
| sort.Slice(newPrType.Components, func(i, j int) bool { | ||
| return newPrType.Components[i].Name < newPrType.Components[j].Name | ||
| }) | ||
| data = append(data, newPrType) | ||
| } | ||
| sort.Slice(data, func(i, j int) bool { | ||
| return data[i].Name < data[j].Name | ||
| }) | ||
| return data | ||
| } | ||
|
|
||
| func writePrInfos(fileout string, prPerType prsByType) (err error) { | ||
| writeTo := os.Stdout | ||
| if fileout != "" { | ||
| writeTo, err = os.OpenFile(fileout, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| data := createSortedPrTypeSlice(prPerType) | ||
|
|
||
| t := template.Must(template.New("markdownTemplate").Parse(markdownTemplate)) | ||
| err = t.ExecuteTemplate(writeTo, "markdownTemplate", data) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func main() { | ||
| from := flag.String("from", "", "from sha/tag/branch") | ||
| to := flag.String("to", "HEAD", "to sha/tag/branch") | ||
| fileout := flag.String("file", "", "file on which to write release notes, stdout if empty") | ||
|
|
||
| flag.Parse() | ||
|
|
||
| prs, err := loadMergedPRs(*from, *to) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
|
|
||
| prInfos, err := loadAllPRs(prs) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
|
|
||
| prPerType := groupPRs(prInfos) | ||
|
|
||
| err = writePrInfos(*fileout, prPerType) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /* | ||
| Copyright 2021 The Vitess Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "vitess.io/vitess/go/test/utils" | ||
| ) | ||
|
|
||
| func Test_groupPRs(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| prInfos []prInfo | ||
| want map[string]map[string][]prInfo | ||
| }{ | ||
| { | ||
| name: "Single PR info with no labels", | ||
| prInfos: []prInfo{{Title: "pr 1", Number: 1}}, | ||
| want: map[string]map[string][]prInfo{"Other": {"Other": []prInfo{{Title: "pr 1", Number: 1}}}}, | ||
| }, { | ||
| name: "Single PR info with type label", | ||
| prInfos: []prInfo{{Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}}}}, | ||
| want: map[string]map[string][]prInfo{"Bug fixes": {"Other": []prInfo{{Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}}}}}}}, | ||
| { | ||
| name: "Single PR info with type and component labels", | ||
| prInfos: []prInfo{{Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}, {Name: prefixComponent + "VTGate"}}}}, | ||
| want: map[string]map[string][]prInfo{"Bug fixes": {"VTGate": []prInfo{{Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}, {Name: prefixComponent + "VTGate"}}}}}}}, | ||
| { | ||
| name: "Multiple PR infos with type and component labels", prInfos: []prInfo{ | ||
| {Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}, {Name: prefixComponent + "VTGate"}}}, | ||
| {Title: "pr 2", Number: 2, Labels: []label{{Name: prefixType + "Feature"}, {Name: prefixComponent + "VTTablet"}}}}, | ||
| want: map[string]map[string][]prInfo{"Bug fixes": {"VTGate": []prInfo{{Title: "pr 1", Number: 1, Labels: []label{{Name: prefixType + "Bug"}, {Name: prefixComponent + "VTGate"}}}}}, "Feature": {"VTTablet": []prInfo{{Title: "pr 2", Number: 2, Labels: []label{{Name: prefixType + "Feature"}, {Name: prefixComponent + "VTTablet"}}}}}}}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got := groupPRs(tt.prInfos) | ||
| utils.MustMatch(t, tt.want, got) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
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 melabel instead?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
¯\_(ツ)_/¯