internal/task: fix PushIssues
I overlooked that we do different things to issues depending on the
release type. For beta1's, we remove okay-after-beta1. For major/minor
releases, we move them to the next milestone and close the current.
For RCs we do nothing. Build all that logic into PushIssues.
For golang/go#51797.
Change-Id: I1faef7bae6b4ec27611a8aa64dcb84c4dba3c0d5
Reviewed-on: https://go-review.googlesource.com/c/build/+/409554
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
diff --git a/internal/task/milestones.go b/internal/task/milestones.go
index bacef5e..4601186 100644
--- a/internal/task/milestones.go
+++ b/internal/task/milestones.go
@@ -26,7 +26,7 @@
KindUnknown ReleaseKind = iota
KindBeta
KindRC
- KindFinal
+ KindMajor
KindMinor
)
@@ -36,7 +36,7 @@
// FetchMilestones returns the milestone numbers for the version currently being
// released, and the next version that outstanding issues should be moved to.
-// If this is a final release, it also creates its first minor release
+// If this is a major release, it also creates its first minor release
// milestone.
func (m *MilestoneTasks) FetchMilestones(ctx *workflow.TaskContext, currentVersion string, kind ReleaseKind) (ReleaseMilestones, error) {
x, ok := goversion.Go1PointX(currentVersion)
@@ -62,7 +62,7 @@
if err != nil {
return ReleaseMilestones{}, err
}
- if kind == KindFinal {
+ if kind == KindMajor {
// Create the first minor release milestone too.
firstMinor := majorVersion + ".1"
if err != nil {
@@ -92,14 +92,18 @@
// CheckBlockers returns an error if there are open release blockers in
// the current milestone.
-func (m *MilestoneTasks) CheckBlockers(ctx *workflow.TaskContext, milestones ReleaseMilestones, kind ReleaseKind) (string, error) {
+func (m *MilestoneTasks) CheckBlockers(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) (string, error) {
issues, err := m.loadMilestoneIssues(ctx, milestones.Current, kind)
if err != nil {
return "", err
}
var blockers []string
- for number, blocker := range issues {
- if blocker {
+ for number, labels := range issues {
+ releaseBlocker := labels["release-blocker"]
+ if kind == KindBeta && (labels["okay-after-beta1"] || !strings.HasSuffix(version, "beta1")) {
+ releaseBlocker = false
+ }
+ if releaseBlocker {
blockers = append(blockers, fmt.Sprintf("https://go.dev/issue/%v", number))
}
}
@@ -110,10 +114,10 @@
return "", nil
}
-// loadMilestoneIssues returns all the open issues in milestone and whether
-// they should block the given kind of release.
-func (m *MilestoneTasks) loadMilestoneIssues(ctx *workflow.TaskContext, milestoneID int, kind ReleaseKind) (map[int]bool, error) {
- issues := map[int]bool{}
+// loadMilestoneIssues returns all the open issues in the specified milestone
+// and their labels.
+func (m *MilestoneTasks) loadMilestoneIssues(ctx *workflow.TaskContext, milestoneID int, kind ReleaseKind) (map[int]map[string]bool, error) {
+ issues := map[int]map[string]bool{}
var query struct {
Repository struct {
Issues struct {
@@ -152,20 +156,11 @@
if issue.Labels.PageInfo.HasNextPage {
return nil, fmt.Errorf("issue %v (#%v) has more than 10 labels", issue.Title, issue.Number)
}
- releaseBlocker := false
- betaOK := false
+ labels := map[string]bool{}
for _, label := range issue.Labels.Nodes {
- if label.Name == "release-blocker" {
- releaseBlocker = true
- }
- if label.Name == "okay-after-beta1" {
- betaOK = true
- }
+ labels[label.Name] = true
}
- if kind == KindBeta && betaOK {
- releaseBlocker = false
- }
- issues[issue.Number] = releaseBlocker
+ issues[issue.Number] = labels
}
if query.Repository.Issues.PageInfo.HasNextPage {
afterToken = &query.Repository.Issues.PageInfo.EndCursor
@@ -174,15 +169,46 @@
return issues, nil
}
-// PushIssues moves all the open issues in the current milestone to the next one.
-func (m *MilestoneTasks) PushIssues(ctx *workflow.TaskContext, milestones ReleaseMilestones) (string, error) {
+// PushIssues updates issues to reflect a finished release. For beta1 releases,
+// it removes the okay-after-beta1 label. For major and minor releases,
+// it moves them to the next milestone and closes the current one.
+func (m *MilestoneTasks) PushIssues(ctx *workflow.TaskContext, milestones ReleaseMilestones, version string, kind ReleaseKind) (string, error) {
+ // For RCs we don't change issues at all.
+ if kind == KindRC {
+ return "", nil
+ }
+
issues, err := m.loadMilestoneIssues(ctx, milestones.Current, KindUnknown)
if err != nil {
return "", err
}
- for issueNumber := range issues {
+ for issueNumber, labels := range issues {
+ var newLabels *[]string
+ var newMilestone *int
+ if kind == KindBeta && strings.HasSuffix(version, "beta1") {
+ if labels["okay-after-beta1"] {
+ newLabels = &[]string{}
+ for label := range labels {
+ if label == "okay-after-beta1" {
+ continue
+ }
+ *newLabels = append(*newLabels, label)
+ }
+ }
+ } else if kind == KindMajor || kind == KindMinor {
+ newMilestone = &milestones.Next
+ }
_, _, err := m.Client.EditIssue(ctx, m.RepoOwner, m.RepoName, issueNumber, &github.IssueRequest{
- Milestone: &milestones.Next,
+ Milestone: newMilestone,
+ Labels: newLabels,
+ })
+ if err != nil {
+ return "", err
+ }
+ }
+ if kind == KindMajor || kind == KindMinor {
+ _, _, err := m.Client.EditMilestone(ctx, m.RepoOwner, m.RepoName, milestones.Current, &github.Milestone{
+ State: github.String("closed"),
})
if err != nil {
return "", err
@@ -203,6 +229,9 @@
// See github.Client.Issues.Edit.
EditIssue(ctx context.Context, owner string, repo string, number int, issue *github.IssueRequest) (*github.Issue, *github.Response, error)
+
+ // See github.Client.Issues.EditMilestone
+ EditMilestone(ctx context.Context, owner string, repo string, number int, milestone *github.Milestone) (*github.Milestone, *github.Response, error)
}
type GitHubClient struct {
@@ -288,3 +317,7 @@
func (c *GitHubClient) EditIssue(ctx context.Context, owner string, repo string, number int, issue *github.IssueRequest) (*github.Issue, *github.Response, error) {
return c.V3.Issues.Edit(ctx, owner, repo, number, issue)
}
+
+func (c *GitHubClient) EditMilestone(ctx context.Context, owner string, repo string, number int, milestone *github.Milestone) (*github.Milestone, *github.Response, error) {
+ return c.V3.Issues.EditMilestone(ctx, owner, repo, number, milestone)
+}
diff --git a/internal/task/milestones_test.go b/internal/task/milestones_test.go
index 0648007..1e06119 100644
--- a/internal/task/milestones_test.go
+++ b/internal/task/milestones_test.go
@@ -40,7 +40,7 @@
client3 := github.NewClient(httpClient)
client4 := githubv4.NewClient(httpClient)
- blocker, err := resetRepo(ctx, client3)
+ normal, blocker, err := resetRepo(ctx, client3)
if err != nil {
t.Fatal(err)
}
@@ -53,63 +53,91 @@
RepoOwner: *flagOwner,
RepoName: *flagRepo,
}
- milestones, err := tasks.FetchMilestones(ctx, "go1.20", KindFinal)
+ milestones, err := tasks.FetchMilestones(ctx, "go1.20", KindMajor)
if err != nil {
t.Fatalf("GetMilestones: %v", err)
}
- _, err = tasks.CheckBlockers(ctx, milestones, KindFinal)
+ if _, err := tasks.PushIssues(ctx, milestones, "go1.20beta1", KindBeta); err != nil {
+ t.Fatalf("Pushing issues for beta release: %v", err)
+ }
+ pushedBlocker, _, err := client3.Issues.Get(ctx, *flagOwner, *flagRepo, blocker.GetNumber())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(pushedBlocker.Labels) != 1 || *pushedBlocker.Labels[0].Name != "release-blocker" {
+ t.Errorf("release blocking issue has labels %#v, should only have release-blocker", pushedBlocker.Labels)
+ }
+ _, err = tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor)
if err == nil || !strings.Contains(err.Error(), "open release blockers") {
t.Fatalf("CheckBlockers with an open release blocker didn't give expected error: %v", err)
}
if _, _, err := client3.Issues.Edit(ctx, *flagOwner, *flagRepo, *blocker.Number, &github.IssueRequest{State: github.String("closed")}); err != nil {
t.Fatal(err)
}
- if _, err := tasks.CheckBlockers(ctx, milestones, KindFinal); err != nil {
+ if _, err := tasks.CheckBlockers(ctx, milestones, "go1.20", KindMajor); err != nil {
t.Fatalf("CheckBlockers with no release blockers failed: %v", err)
}
+ if _, err := tasks.PushIssues(ctx, milestones, "go1.20", KindMajor); err != nil {
+ t.Fatalf("PushIssues for major release failed: %v", err)
+ }
+ milestone, _, err := client3.Issues.GetMilestone(ctx, *flagOwner, *flagRepo, milestones.Current)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if milestone.GetState() != "closed" {
+ t.Errorf("current milestone is %q, should be closed", milestone.GetState())
+ }
+ pushedNormal, _, err := client3.Issues.Get(ctx, *flagOwner, *flagRepo, normal.GetNumber())
+ if err != nil {
+ t.Fatal(err)
+ }
+ if pushedNormal.GetMilestone().GetNumber() != milestones.Next {
+ t.Errorf("issue %v is on milestone %v, should have been pushed to %v", normal.GetNumber(), pushedNormal.GetMilestone().GetNumber(), milestones.Next)
+ }
}
// resetRepo clears out the test repository and sets it to have:
// - a single milestone, Go1.20
// - a normal issue in that milestone
-// - a release blocking issue in that milestone, which is returned.
-func resetRepo(ctx context.Context, client *github.Client) (*github.Issue, error) {
- milestones, _, err := client.Issues.ListMilestones(ctx, *flagOwner, *flagRepo, nil)
+// - an okay-after-beta1 release blocking issue in that milestone, which is returned.
+func resetRepo(ctx context.Context, client *github.Client) (normal, blocker *github.Issue, err error) {
+ milestones, _, err := client.Issues.ListMilestones(ctx, *flagOwner, *flagRepo, &github.MilestoneListOptions{State: "all"})
if err != nil {
- return nil, err
+ return nil, nil, err
}
for _, m := range milestones {
if _, err := client.Issues.DeleteMilestone(ctx, *flagOwner, *flagRepo, *m.Number); err != nil {
- return nil, err
+ return nil, nil, err
}
}
issues, _, err := client.Issues.ListByRepo(ctx, *flagOwner, *flagRepo, nil)
if err != nil {
- return nil, err
+ return nil, nil, err
}
for _, i := range issues {
if _, _, err := client.Issues.Edit(ctx, *flagOwner, *flagRepo, *i.Number, &github.IssueRequest{
State: github.String("CLOSED"),
}); err != nil {
- return nil, err
+ return nil, nil, err
}
}
currentMilestone, _, err := client.Issues.CreateMilestone(ctx, *flagOwner, *flagRepo, &github.Milestone{Title: github.String("Go1.20")})
if err != nil {
- return nil, err
+ return nil, nil, err
}
- if _, _, err := client.Issues.Create(ctx, *flagOwner, *flagRepo, &github.IssueRequest{
+ normal, _, err = client.Issues.Create(ctx, *flagOwner, *flagRepo, &github.IssueRequest{
Title: github.String("Non-release-blocker"),
Milestone: currentMilestone.Number,
- }); err != nil {
- return nil, err
+ })
+ if err != nil {
+ return nil, nil, err
}
- blocker, _, err := client.Issues.Create(ctx, *flagOwner, *flagRepo, &github.IssueRequest{
+ blocker, _, err = client.Issues.Create(ctx, *flagOwner, *flagRepo, &github.IssueRequest{
Title: github.String("Release-blocker"),
Milestone: currentMilestone.Number,
- Labels: &[]string{"release-blocker"},
+ Labels: &[]string{"release-blocker", "okay-after-beta1"},
})
- return blocker, err
+ return normal, blocker, err
}
type testLogger struct {
