cmd/gopherbot: remove sequential order assumption

Previously we assumed that Gerrit issued CL numbers in sequential
order: If it issued CL N, any new CL would be in the range [n+1, ∞).
However, observation shows this not to be true; I submitted a number
of CL's just now and got this ordering: 49910, 49911, 49891, 49852.

This explains why there are ten people who have yet to be
congratulated for submitting their first CL: Gerrit issued a newer
CL number and we ignored any CL's older than that. I didn't catch
this in testing because I wasn't running in daemon mode locally, so
mostRecentCL was always 0.

Change-Id: I822ca12a093bd6c186701a4d49b47b2671bcee8a
Reviewed-on: https://go-review.googlesource.com/49853
Reviewed-by: Jessie Frazelle <me@jessfraz.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

cmd/gopherbot/gopherbot.go

@@ -189,8 +189,6 @@ type gopherbot struct {
 	// github issues. It's updated at the end of the run of tasks.
 	maxIssueMod       time.Time
 	knownContributors map[string]bool
-	// Most recent CL processed by the contributor congratulation script
-	mostRecentCL int32
 }
 
 var tasks = []struct {
@@ -763,24 +761,21 @@ var congratsEpoch = time.Date(2017, 6, 17, 0, 0, 0, 0, time.UTC)
 
 func (b *gopherbot) congratulateNewContributors(ctx context.Context) error {
 	cls := make(map[string]*maintner.GerritCL)
-	newHighestCL := b.mostRecentCL
 	b.corpus.Gerrit().ForeachProjectUnsorted(func(gp *maintner.GerritProject) error {
 		if gp.Server() != "go.googlesource.com" {
 			return nil
 		}
 		return gp.ForeachCLUnsorted(func(cl *maintner.GerritCL) error {
+			// CLs can be returned by maintner in any order. Note also that
+			// Gerrit CL numbers are sparse (CL N does not guarantee that CL N-1
+			// exists) and Gerrit issues CL's out of order - it may issue CL N,
+			// then CL (N - 18), then CL (N - 40).
 			if b.knownContributors == nil {
 				b.knownContributors = make(map[string]bool)
 			}
 			if cl.Commit == nil {
 				return nil
 			}
-			if cl.Number <= b.mostRecentCL {
-				return nil
-			}
-			if cl.Number > newHighestCL {
-				newHighestCL = cl.Number
-			}
 			email := cl.Commit.Author.Str
 			if b.knownContributors[email] {
 				return nil
@@ -839,7 +834,6 @@ func (b *gopherbot) congratulateNewContributors(ctx context.Context) error {
 		}
 		b.knownContributors[email] = true
 	}
-	b.mostRecentCL = newHighestCL
 	return nil
 }
 

maintner/gerrit.go

@@ -210,8 +210,10 @@ type GerritCL struct {
 	// Project is the project this CL is part of.
 	Project *GerritProject
 
-	// Number is the CL number on the Gerrit
-	// server. (e.g. 1, 2, 3)
+	// Number is the CL number on the Gerrit server (e.g. 1, 2, 3). Gerrit CL
+	// numbers are sparse (CL N does not guarantee that CL N-1 exists) and
+	// Gerrit issues CL's out of order - it may issue CL N, then CL (N - 18),
+	// then CL (N - 40).
 	Number int32
 
 	Created time.Time