maintner: remove nil check in GerritCL.Subject method
In CL 107296, the maintner API had undergone a change such that all
public endpoints would only return GerritCLs that are considered
complete. Complete CLs are defined to have certain properties that
make working with them more friendly. One of those properties is that
a complete GerritCL always has a non-nil Commit field.
Methods of GerritCL type are expected to operate on complete CLs
that were acquired via one of the public means (e.g., ForeachOpenCL,
ForeachCLUnsorted, CL), so it's a precondition that the CL must be
complete. (If the user makes a GerritCL type from scratch, they are
expected to follow the documented requirement that GerritCL.Commit
must be non-nil.)
The Footer, ChangeID methods were already relying on the fact that
Commit is non-nil. This change updates the GerritCL.Subject method
to also make use of that property, which makes the cl.Commit nil
check not necessary.
This leads to simpler code and higher confidence that our internal
invariants are maintained.
Change-Id: I5867fd248274b8dd94a0ca98a91a773d48b3bdec
Reviewed-on: https://go-review.googlesource.com/c/161221
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/maintner/gerrit.go b/maintner/gerrit.go
index ffd03ca..b007932 100644
--- a/maintner/gerrit.go
+++ b/maintner/gerrit.go
@@ -489,9 +489,6 @@
// Subject returns the first line of the latest commit message.
func (cl *GerritCL) Subject() string {
- if cl.Commit == nil {
- return ""
- }
if i := strings.Index(cl.Commit.Msg, "\n"); i >= 0 {
return cl.Commit.Msg[:i]
}
diff --git a/maintner/gerrit_test.go b/maintner/gerrit_test.go
index ba227fd..691ae25 100644
--- a/maintner/gerrit_test.go
+++ b/maintner/gerrit_test.go
@@ -226,23 +226,31 @@
}
func TestSubject(t *testing.T) {
- cl := &GerritCL{}
- if w, e := cl.Subject(), ""; w != e {
- t.Errorf("cl.Subject() = %q; want %q", w, e)
- }
-
testcases := []struct{ msg, subject string }{
{"maintner: slurp up all the things", "maintner: slurp up all the things"},
{"cmd/go: build stuff\n\nand do other stuff, too.", "cmd/go: build stuff"},
}
for _, tc := range testcases {
- cl = &GerritCL{Commit: &GitCommit{Msg: tc.msg}}
+ cl := &GerritCL{Commit: &GitCommit{Msg: tc.msg}}
if cl.Subject() != tc.subject {
t.Errorf("cl.Subject() = %q; want %q", cl.Subject(), tc.subject)
}
}
}
+func TestChangeID(t *testing.T) {
+ testcases := []struct{ msg, changeID string }{
+ {"maintner: slurp up all the things", ""},
+ {"cmd/go: build stuff\n\nChange-Id: I7d3850e6774403c5d4ae15ca94c31c2f46f4ffa3", "I7d3850e6774403c5d4ae15ca94c31c2f46f4ffa3"},
+ }
+ for _, tc := range testcases {
+ cl := &GerritCL{Commit: &GitCommit{Msg: tc.msg}}
+ if cl.ChangeID() != tc.changeID {
+ t.Errorf("cl.ChangeID() = %q; want %q", cl.ChangeID(), tc.changeID)
+ }
+ }
+}
+
func TestLineValueOK(t *testing.T) {
tests := []struct {
all, prefix, want, wantRest string
