Skip to content

Commit 9187b9e

Browse files
committed
maintner/maintnerd: change definition of Go release to require release branch
Previously, the list-Go-releases RPC endpoint was more flexible and considered a Go release to exist as long as there's a git tag for it, regardless of whether a corresponding release branch existed. Such a situation is very unlikely to come up, and we don't want to cause all users of this API to filter out releases without a release branch. So just re-define a Go release to require a release branch, and make that the API promise. In practice, this should not have any effect because all go tags have corresponding release branches. Updates golang/go#17626 Change-Id: Ia7a8354000483c969e123f0f3605fd360846c40b Reviewed-on: https://go-review.googlesource.com/c/147200 Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 1cecfeb commit 9187b9e

File tree

4 files changed

+20
-10
lines changed

4 files changed

+20
-10
lines changed

maintner/maintnerd/apipb/api.pb.go

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

maintner/maintnerd/apipb/api.proto

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ message GoRelease {
7979
string tag_commit = 5; // "26957168c4c0cdcc7ca4f0b19d0eb19474d224ac"
8080

8181
// Release branch information for this major-minor version pair.
82-
// Empty if the corresponding release branch doesn't exist.
8382
string branch_name = 6; // "release-branch.go1.11", etc.
8483
string branch_commit = 7; // most recent commit on the release branch, e.g., "edb6c16b9b62ed8586d2e3e422911d646095b7e5"
8584
}
@@ -97,7 +96,8 @@ service MaintnerService {
9796
// GoFindTryWork finds trybot work for the coordinator to build & test.
9897
rpc GoFindTryWork(GoFindTryWorkRequest) returns (GoFindTryWorkResponse);
9998

100-
// ListGoReleases lists Go releases. A release is considered to exist
101-
// if a tag for it exists.
99+
// ListGoReleases lists Go releases. A release is considered to exist for
100+
// each git tag named "goX", "goX.Y", or "goX.Y.Z", as long as it has a
101+
// corresponding "release-branch.goX" or "release-branch.goX.Y" release branch.
102102
rpc ListGoReleases(ListGoReleasesRequest) returns (ListGoReleasesResponse);
103103
}

maintner/maintnerd/maintapi/api.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,16 @@ func supportedGoReleases(goProj nonChangeRefLister) ([]*apipb.GoRelease, error)
302302
return nil, err
303303
}
304304

305-
// Releases are considered only to exist if they've been tagged.
305+
// A release is considered to exist for each git tag named "goX", "goX.Y", or "goX.Y.Z",
306+
// as long as it has a corresponding "release-branch.goX" or "release-branch.goX.Y" release branch.
306307
var rs []*apipb.GoRelease
307308
for v, t := range tags {
308-
b := branches[v]
309+
b, ok := branches[v]
310+
if !ok {
311+
// In the unlikely case a tag exists but there's no release branch for it,
312+
// don't consider it a release. This way, callers won't have to do this work.
313+
continue
314+
}
309315
rs = append(rs, &apipb.GoRelease{
310316
Major: v.Major,
311317
Minor: v.Minor,

maintner/maintnerd/maintapi/api_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,9 @@ func TestSupportedGoReleases(t *testing.T) {
258258
{"refs/heads/release-branch.go1.42", gitHash("362986e7a4b5edc911ed55324c37106c40abe3fb")},
259259
{"refs/heads/release-branch.go2", gitHash("cfbe0f14bcbf1e773f8dd9a968c80cf0b9238c59")},
260260
{"refs/heads/release-branch.go1.2", gitHash("6523e1eb33ef792df04e08462ed332b95311261e")},
261+
262+
// It doesn't count as a release if there's no corresponding release-branch.go1.43 release branch.
263+
{"refs/tags/go1.43", gitHash("3aa7f7065ecf717b1dd6512bb7a9f40625fc8cb5")},
261264
},
262265
},
263266
want: []*apipb.GoRelease{

0 commit comments

Comments
 (0)