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

Hierarchical release note feature functions #435

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion toolbox/relnotes/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = ["main.go"],
srcs = [
"hierarchy.go",
"main.go",
],
importpath = "k8s.io/release/toolbox/relnotes",
visibility = ["//visibility:private"],
deps = [
Expand Down
196 changes: 196 additions & 0 deletions toolbox/relnotes/hierarchy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package main

import (
"context"
"fmt"
"os"
"regexp"
"sort"
"strconv"
"strings"

"github.com/google/go-github/github"
u "k8s.io/release/toolbox/util"
)

type dictSIG map[string]dictArea
type dictArea map[string]dictIssue
type dictIssue map[int]dictPR
type dictPR map[int]bool

func hierarchicalNoteLayout(f *os.File, dict dictSIG, prMap map[int]*github.Issue, g *u.GithubClient, owner, repo string) error {
// Sort and iterate through sigs
var keySigs []string
for sig := range dict {
keySigs = append(keySigs, sig)
}
sort.Strings(keySigs)
for _, sig := range keySigs {
areas := dict[sig]
f.WriteString(fmt.Sprintf("### %s\n\n", strings.Title(sig)))

// Sort and iterate through areas
var keyAreas []string
for area := range areas {
keyAreas = append(keyAreas, area)
}
sort.Strings(keyAreas)
for _, area := range keyAreas {
issues := areas[area]
f.WriteString(fmt.Sprintf("#### %s\n\n", strings.Title(area)))

for issueID, prs := range issues {
if issueID >= 0 {
issue, _, err := g.GetIssue(context.Background(), owner, repo, issueID)
if err != nil {
return err
}
f.WriteString(fmt.Sprintf("* %s (#%d)\n\n", *issue.Title, issueID))
} else {
f.WriteString(fmt.Sprintf("* NullIssue\n\n"))
}
for pr := range prs {
f.WriteString(fmt.Sprintf(" * %s (#%d, @%s)\n", extractReleaseNoteFromPR(prMap[pr]), pr, *prMap[pr].User.Login))
}
f.WriteString("\n")
}
}
}
return nil
}

// createHierarchicalNote given release PRs and issue map, creates hierarchical release note
// map[SIG]map[Area]map[Issue]PR.
func createHierarchicalNote(prs []int, prMap map[int]*github.Issue, g *u.GithubClient, owner, repo string) (dictSIG, error) {
var dict = dictSIG{}

for _, pr := range prs {
issues, err := extractFixedIssues(*prMap[pr].Body, owner, repo, g)
if err != nil {
return nil, fmt.Errorf("failed to extract fixed issues: %v", err)
}
if len(issues) == 0 {
// In our design doc, the automation should enforce every release-note PR
// with at least one issue. However it's observed that the rule is not
// applied yet. Also old PRs may not link to an issue.
//
// To produce info-richer release note, we try to get SIG and Area label
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, most of PRs don't have either sig or area label. I once felt a bit painful when I wanted to filter PRs by sig. I was thinking we enforce sig labels on PRs as we already do on issues. Then we may get more info here. @enisoc @Bradamant3 WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Enforcing sig label on PR will also be helpful for #348.

// from PRs which don't link to any issue.
issues = append(issues, prMap[pr])
}
for _, issue := range issues {
sigs := extractIssueSIGs(issue)
area := extractIssueArea(issue)
i := *issue.Number
// For PRs that don't link to any issue, restore the nullIssue information
if issue.IsPullRequest() {
i = -1
}
if len(sigs) == 0 {
setNoteDict(dict, "nullSig", area, i, pr)
continue
}
for _, s := range sigs {
setNoteDict(dict, s, area, i, pr)
}
}
}

return dict, nil
}

// setNoteDict sets the entry dict[sig][area][issue][pr] to be true, initializes nil maps along
// the way.
func setNoteDict(dict dictSIG, sig, area string, issue, pr int) {
if dict[sig] == nil {
dict[sig] = dictArea{}
}
if dict[sig][area] == nil {
dict[sig][area] = dictIssue{}
}
if dict[sig][area][issue] == nil {
dict[sig][area][issue] = dictPR{}
}
dict[sig][area][issue][pr] = true
}

// extractFixedIssues parses the fixed issues' id from PR body.
func extractFixedIssues(msg, owner, repo string, g *u.GithubClient) ([]*github.Issue, error) {
var issueIDs = make([]int, 0)
var issues = make([]*github.Issue, 0)
// Captures "fixes #<issue id>" and "fixes: #<issue id>"
re, _ := regexp.Compile("fixes:* #([0-9]+)")
matches := re.FindAllStringSubmatch(strings.ToLower(msg), -1)
for _, match := range matches {
id, _ := strconv.Atoi(match[1])
issueIDs = append(issueIDs, id)
}

// Captures "ref #<issue id>" and "ref: #<issue id>"
re, _ = regexp.Compile("ref:* #([0-9]+)")
matches = re.FindAllStringSubmatch(strings.ToLower(msg), -1)
for _, match := range matches {
id, _ := strconv.Atoi(match[1])
issueIDs = append(issueIDs, id)
}

// Captures "fixes https://github.com/kubernetes/kubernetes/issues/<issue id>" and "fixes: https://github.com/kubernetes/kubernetes/issues/<issue id>"
re, _ = regexp.Compile("fixes:* https://github.com/kubernetes/kubernetes/issues/([0-9]+)")
matches = re.FindAllStringSubmatch(strings.ToLower(msg), -1)
for _, match := range matches {
id, _ := strconv.Atoi(match[1])
issueIDs = append(issueIDs, id)
}

// Captures "ref https://github.com/kubernetes/kubernetes/issues/<issue id>" and "ref: https://github.com/kubernetes/kubernetes/issues/<issue id>"
re, _ = regexp.Compile("ref:* https://github.com/kubernetes/kubernetes/issues/([0-9]+)")
matches = re.FindAllStringSubmatch(strings.ToLower(msg), -1)
for _, match := range matches {
id, _ := strconv.Atoi(match[1])
issueIDs = append(issueIDs, id)
}

// Extract issues
for _, id := range issueIDs {
issue, _, err := g.GetIssue(context.Background(), owner, repo, id)
if err != nil {
return nil, fmt.Errorf("failed to get issue %d: %v", id, err)
}
// Get rid of PRs
if issue.IsPullRequest() {
continue
}
issues = append(issues, issue)
}

return issues, nil
}

// extractIssueSIGs gets the SIGs of the input issue (if there is any)
func extractIssueSIGs(i *github.Issue) []string {
var sigs = make([]string, 0)
for _, l := range i.Labels {
if strings.HasPrefix(*l.Name, "sig/") {
sigs = append(sigs, (*l.Name)[4:])
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using TrimPrefix here?

}
}

return sigs
}

// extractIssueSIGs gets the Areas of the input issue and returns as a single string. If the issue
// doesn't have any Area label, the function returns "nullArea".
func extractIssueArea(i *github.Issue) string {
var areas = make([]string, 0)
for _, l := range i.Labels {
if strings.HasPrefix(*l.Name, "area/") {
areas = append(areas, (*l.Name)[5:])

Choose a reason for hiding this comment

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

Does github api guarantee anything about the order of the labels returned? I am wondering because if two issues have labels "area/Alpha" and "area/Beta" and the labels can be in any order, then they might be put into different area buckets in the final document, even though they have the same areas

Copy link
Member Author

@roycaihw roycaihw Oct 11, 2017

Choose a reason for hiding this comment

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

@jennybuckley I think it guarantees alphabetical order.

Copy link
Contributor

@xiangpengzhao xiangpengzhao Oct 20, 2017

Choose a reason for hiding this comment

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

Also TrimPrefix here.

}
}

if len(areas) == 0 {
return "nullArea"
}

return strings.Join(areas, " & ")
}
27 changes: 19 additions & 8 deletions toolbox/relnotes/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func main() {

// Generating release note...
log.Print("Generating release notes...")
err = gatherPRNotes(prFileName, releaseInfo)
err = gatherPRNotes(client, prFileName, releaseInfo)
if err != nil {
log.Printf("failed to gather PR notes: %v", err)
os.Exit(1)
Expand Down Expand Up @@ -271,7 +271,7 @@ func gatherReleaseInfo(g *u.GithubClient, branchRange string) (*ReleaseInfo, err
return &info, nil
}

func gatherPRNotes(prFileName string, info *ReleaseInfo) error {
func gatherPRNotes(g *u.GithubClient, prFileName string, info *ReleaseInfo) error {
var result error
prFile, err := os.Create(prFileName)
if err != nil {
Expand All @@ -289,7 +289,7 @@ func gatherPRNotes(prFileName string, info *ReleaseInfo) error {
changelogURL := fmt.Sprintf("%s%s/%s/master/CHANGELOG%s.md", u.GithubRawURL, *owner, *repo, branchVerSuffix)
minorRelease(prFile, info.releaseTag, draftURL, changelogURL)
} else {
patchRelease(prFile, info)
patchRelease(g, prFile, info)
}
return result
}
Expand Down Expand Up @@ -620,27 +620,38 @@ func minorRelease(f *os.File, release, draftURL, changelogURL string) {
}

// patchRelease performs a patch (vX.Y.Z) release by printing out all the related changes.
func patchRelease(f *os.File, info *ReleaseInfo) {
func patchRelease(g *u.GithubClient, f *os.File, info *ReleaseInfo) error {
// Release note for different labels
f.WriteString(fmt.Sprintf("## Changelog since %s\n\n", info.startTag))

if len(info.releaseActionRequiredPRs) > 0 {
f.WriteString("### Action Required\n\n")
for _, pr := range info.releaseActionRequiredPRs {
f.WriteString(fmt.Sprintf("* %s (#%d, @%s)\n", extractReleaseNoteFromPR(info.prMap[pr]), pr, *info.prMap[pr].User.Login))
dict, err := createHierarchicalNote(info.releaseActionRequiredPRs, info.prMap, g, *owner, *repo)
if err != nil {
return fmt.Errorf("failed to create action required hierarchical note: %v", err)
}
err = hierarchicalNoteLayout(f, dict, info.prMap, g, *owner, *repo)
if err != nil {
return fmt.Errorf("failed to layout action required hierarchical note: %v", err)
}
f.WriteString("\n")
}

if len(info.releasePRs) > 0 {
f.WriteString("### Other notable changes\n\n")
for _, pr := range info.releasePRs {
f.WriteString(fmt.Sprintf("* %s (#%d, @%s)\n", extractReleaseNoteFromPR(info.prMap[pr]), pr, *info.prMap[pr].User.Login))
dict, err := createHierarchicalNote(info.releasePRs, info.prMap, g, *owner, *repo)
if err != nil {
return fmt.Errorf("failed to create normal hierarchical note: %v", err)
}
err = hierarchicalNoteLayout(f, dict, info.prMap, g, *owner, *repo)
if err != nil {
return fmt.Errorf("failed to layout normal hierarchical note: %v", err)
}
f.WriteString("\n")
} else {
f.WriteString("**No notable changes for this release**\n\n")
}
return nil
}

// extractReleaseNoteFromPR tries to fetch release note from PR body, otherwise uses PR title.
Expand Down
5 changes: 5 additions & 0 deletions toolbox/util/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,8 @@ func AddQuery(query []string, queryParts ...string) []string {
func (g GithubClient) GetBranch(ctx context.Context, owner, repo, branch string) (*github.Branch, *github.Response, error) {
return g.client.Repositories.GetBranch(ctx, owner, repo, branch)
}

// GetIssue is a wrapper of Github issue Get function.
func (g GithubClient) GetIssue(ctx context.Context, owner, repo string, id int) (*github.Issue, *github.Response, error) {
return g.client.Issues.Get(ctx, owner, repo, id)
}