Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/review/git-codereview: shouldn't accept empty change commit message #9336

Closed
bradfitz opened this issue Dec 16, 2014 · 2 comments
Closed

x/review/git-codereview: shouldn't accept empty change commit message #9336

bradfitz opened this issue Dec 16, 2014 · 2 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@bradfitz
Copy link
Contributor

Repro:

-- be on master
-- hack
-- git add all the files
-- git change branch_name
-- (editor comes up)
-- exit editor without writing a message (realizing you screwed up from the message in comments)

Now it leaves you on the (new) branch, but accepts your empty commit message with a newly-added Change-Id.

mac:rand bradfitz$ git br
  apicheck
  apifix
* crypto_eagain
  httptransportclose
  master
  test
  textprotodocs

mac:rand bradfitz$ git show --stat
commit 2926b9ec5d97596c0382162e0b5014dfffa08426
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue Dec 16 15:11:43 2014 +1100

    Change-Id: Iacd608ba43332008984aa8ece17dcb5757f27b3f

 src/crypto/rand/eagain.go    | 27 +++++++++++++++++++++++++++
 src/crypto/rand/rand_unix.go | 18 +++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

It should fail when it sees an empty commit message, instead of creating the branch and committing it.

/cc @rsc @adg
@bradfitz
Copy link
Contributor Author

Andrew and I note the old code did it so clearly right here:

                undef $/;
                open(I, $MSG); $_ = <I>; close I;
                s|^diff --git a/.*||ms;
                s|^#.*$||mg;
                exit unless $_;

@adg
Copy link
Contributor

adg commented Dec 16, 2014

Mailed https://go-review.googlesource.com/1612

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@mikioh mikioh changed the title review: shouldn't accept empty change commit message x/review/git-codereview: shouldn't accept empty change commit message Aug 17, 2015
@rsc rsc closed this as completed Jan 29, 2016
@golang golang locked and limited conversation to collaborators Feb 3, 2017
AndersonQ pushed a commit to AndersonQ/go that referenced this issue Oct 12, 2019
@adg
git-review: reject empty messages in commit-msg hook
1458e6c 
Fixes golang#9336

Change-Id: Ie145c116ed91cfac0eb2061c448b08c5b9595d19
Reviewed-on: https://go-review.googlesource.com/1612
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@bradfitz @rsc @adg @gopherbot