Skip to content

Commit dab65e9

Browse files
committed
[release-branch.go1.11] godoc/redirect: improve Rietveld CL heuristic
Go's CL numbers as assigned by Gerrit have started to collide with the lower numbers in the sparse set of CL numbers as returned by our old code review system (Rietveld). The old heuristic no longer works now that Gerrit CL numbers have reached 150000. Instead, include a map of the low Rietveld CL numbers where we might overlap and bump the threshold heuristic up. Updates golang/go#28836 Change-Id: Ice719ab807ce3922b885a800ac873cdbf165a8f7 Reviewed-on: https://go-review.googlesource.com/c/150057 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> (cherry picked from commit e77c068) Reviewed-on: https://go-review.googlesource.com/c/150577
1 parent f1c3f97 commit dab65e9

File tree

3 files changed

+1110
-4
lines changed

3 files changed

+1110
-4
lines changed

godoc/redirect/redirect.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ var redirects = map[string]string{
115115
"/tour": "http://tour.golang.org",
116116
"/wiki": "https://github.com/golang/go/wiki",
117117

118-
"/doc/articles/c_go_cgo.html": "/blog/c-go-cgo",
118+
"/doc/articles/c_go_cgo.html": "/blog/c-go-cgo",
119119
"/doc/articles/concurrency_patterns.html": "/blog/go-concurrency-patterns-timing-out-and",
120120
"/doc/articles/defer_panic_recover.html": "/blog/defer-panic-and-recover",
121121
"/doc/articles/error_handling.html": "/blog/error-handling-and-go",
@@ -191,9 +191,13 @@ func clHandler(w http.ResponseWriter, r *http.Request) {
191191
return
192192
}
193193
target := ""
194-
// the first CL in rietveld is about 152046, so only treat the id as
195-
// a rietveld CL if it is larger than 150000.
196-
if n, err := strconv.Atoi(id); err == nil && n > 150000 {
194+
195+
if n, err := strconv.Atoi(id); err == nil && isRietveldCL(n) {
196+
// TODO: Issue 28836: if this Rietveld CL happens to
197+
// also be a Gerrit CL, render a disambiguation HTML
198+
// page with two links instead. We'll need to make an
199+
// RPC (to maintner?) to figure that out. For now just
200+
// redirect to rietveld.
197201
target = "https://codereview.appspot.com/" + id
198202
} else {
199203
target = "https://go-review.googlesource.com/" + id

godoc/redirect/redirect_test.go

+9
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ func TestRedirects(t *testing.T) {
6262
"/cl/1/": {302, "https://go-review.googlesource.com/1"},
6363
"/cl/267120043": {302, "https://codereview.appspot.com/267120043"},
6464
"/cl/267120043/": {302, "https://codereview.appspot.com/267120043"},
65+
66+
// Verify that we're using the Rietveld CL table:
67+
"/cl/152046": {302, "https://codereview.appspot.com/152046"},
68+
"/cl/152047": {302, "https://go-review.googlesource.com/152047"},
69+
"/cl/152048": {302, "https://codereview.appspot.com/152048"},
70+
71+
// And verify we're using the the "bigEnoughAssumeRietveld" value:
72+
"/cl/299999": {302, "https://go-review.googlesource.com/299999"},
73+
"/cl/300000": {302, "https://codereview.appspot.com/300000"},
6574
}
6675

6776
mux := http.NewServeMux()

0 commit comments

Comments
 (0)