Skip to content

Commit

Permalink
git-codereview: add support for DΟ NΟT MAIL
Browse files Browse the repository at this point in the history
Also create a local DΟ NΟT SUBMIT check,
although the Gerrit server is also taking care of that.

While we're here, make fixup! and squash! commits
non-mailable as well.

(In the all caps text above the big Os are really Omicrons.
Otherwise the CL would not be mailable or submittable.)

Change-Id: Id1a9806b0d395d1a0bfc50ed7d7d22c64a48a2f1
Reviewed-on: https://go-review.googlesource.com/76877
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
rsc committed Nov 16, 2017
1 parent 69da225 commit 0d52c01
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 1 deletion.
4 changes: 4 additions & 0 deletions git-codereview/doc.go
Expand Up @@ -212,6 +212,10 @@ If there are multiple pending commits, the revision argument is mandatory.
If no revision is specified, the mail command prints a short summary of
the pending commits for use in deciding which to mail.
If any commit that would be pushed to the server contains the text
"DO NOT MAIL" (case insensitive) in its commit message, the mail command
will refuse to send the commit to the server.
Pending
The pending command prints to standard output the status of all pending changes
Expand Down
23 changes: 23 additions & 0 deletions git-codereview/mail.go
Expand Up @@ -51,6 +51,29 @@ func cmdMail(args []string) {
dief("cannot mail: commit %s is empty", c.ShortHash)
}

foundCommit := false
for _, c1 := range b.Pending() {
if c1 == c {
foundCommit = true
}
if !foundCommit {
continue
}
if strings.Contains(strings.ToLower(c1.Message), "do not mail") {
dief("%s: CL says DO NOT MAIL", c1.ShortHash)
}
if strings.HasPrefix(c1.Message, "fixup!") {
dief("%s: CL is a fixup! commit", c1.ShortHash)
}
if strings.HasPrefix(c1.Message, "squash!") {
dief("%s: CL is a squash! commit", c1.ShortHash)
}
}
if !foundCommit {
// b.CommitByRev and b.DefaultCommit both return a commit on b.
dief("internal error: did not find chosen commit on current branch")
}

if !*force && HasStagedChanges() {
dief("there are staged changes; aborting.\n"+
"Use '%s change' to include them or '%s mail -f' to force it.", os.Args[0], os.Args[0])
Expand Down
27 changes: 27 additions & 0 deletions git-codereview/mail_test.go
Expand Up @@ -30,6 +30,33 @@ func TestMail(t *testing.T) {
"git tag -f work.mailed "+h)
}

func TestDoNotMail(t *testing.T) {
gt := newGitTest(t)
defer gt.done()
gt.work(t)
trun(t, gt.client, "git", "commit", "--amend", "-m", "This is my commit.\n\nDO NOT MAIL\n")

testMainDied(t, "mail")
testPrintedStderr(t, "DO NOT MAIL")

trun(t, gt.client, "git", "commit", "--amend", "-m", "fixup! This is my commit.")

testMainDied(t, "mail")
testPrintedStderr(t, "fixup! commit")

trun(t, gt.client, "git", "commit", "--amend", "-m", "squash! This is my commit.")

testMainDied(t, "mail")
testPrintedStderr(t, "squash! commit")

trun(t, gt.client, "git", "commit", "--amend", "-m", "This is my commit.\n\nDO NOT MAIL\n")

// Do not mail even when the DO NOT MAIL is a parent of the thing we asked to mail.
gt.work(t)
testMainDied(t, "mail", "HEAD")
testPrintedStderr(t, "DO NOT MAIL")
}

func TestMailGitHub(t *testing.T) {
gt := newGitTest(t)
defer gt.done()
Expand Down
4 changes: 4 additions & 0 deletions git-codereview/submit.go
Expand Up @@ -78,6 +78,10 @@ func cmdSubmit(args []string) {
// submit submits a single commit c on branch b and returns the
// GerritChange for the submitted change. It dies if the submit fails.
func submit(b *Branch, c *Commit) *GerritChange {
if strings.Contains(strings.ToLower(c.Message), "do not submit") {
dief("%s: CL says DO NOT SUBMIT", c.ShortHash)
}

// Fetch Gerrit information about this change.
g, err := b.GerritChange(c, "LABELS", "CURRENT_REVISION")
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion git-codereview/submit_test.go
Expand Up @@ -23,7 +23,13 @@ func TestSubmitErrors(t *testing.T) {
testPrintedStderr(t, "cannot submit: no changes pending")
write(t, gt.client+"/file1", "")
trun(t, gt.client, "git", "add", "file1")
trun(t, gt.client, "git", "commit", "-m", "msg\n\nChange-Id: I123456789\n")
trun(t, gt.client, "git", "commit", "-m", "msg\n\nDO NOT SUBMIT\n\nChange-Id: I123456789\n")

// Gerrit checks this too, but add a local check.
t.Logf("> do not submit")
testMainDied(t, "submit")
testPrintedStderr(t, "DO NOT SUBMIT")
trun(t, gt.client, "git", "commit", "--amend", "-m", "msg\n\nChange-Id: I123456789\n")

t.Logf("> staged changes")
write(t, gt.client+"/file1", "asdf")
Expand Down

0 comments on commit 0d52c01

Please sign in to comment.