My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 606: Accept Change-Id: tag at any place within commit message
27 people starred this issue and may be notified of changes. Back to list
Status:  New
Owner:  ----


Sign in to add a comment
 
Reported by wal...@google.com, Jun 22, 2010
I am seeing confusion where people add various kinds of information towards the end of their commit messages:

- bug IDs
- Signed-off-by and similar tags
- Describe how they tested the commit
- gerrit Change-Id: tag

All these things tend to be pushed towards the end of a commit message, but not necessarily in a precise order. But gerrit only considers the Change-Id: tag if it's part of the last paragraph, so it will silently ignore it if it's placed somewhere else, and people get confused.

Could gerrit be made to accept Change-Id: lines anywhere in the commit message ?

Jul 27, 2010
#1 antony.s...@gmail.com
It would also cause new changes to generated, when instead a new patchset should be generated, and the changeid line moved in a subsequent patchset.
Jul 27, 2010
#2 wal...@google.com
Antony - I'm not sure I understand your comment.

What I am seeing *now* is that new changes are generated instead of new patchsets, because gerrit ignores the Change-Id: header when it's not in the last paragraph.
Jul 27, 2010
#3 antony.s...@gmail.com
Yah, what you said :) 

I was just trying to add that, the author should be given a change to correct his mistake, without causing a new "change".
Aug 17, 2010
Project Member #4 mf...@codeaurora.org
While it will not stop the problem from happening in the first place (as this issues is suggesting), Issue 653 suggests a method to deal with this problem after the fact.
Feb 7, 2011
#5 em...@pete-woods.com
I'm sure there is a bug in JGit at the minute to do with reading comment tags (if that's the right name). I remember having a quick look into it, but not having time to make a fix.
Feb 7, 2011
Project Member #6 bklarson@gmail.com
I don't think it is a good idea to allow a Change-Id tag anywhere in the commit message.  Having a commit footer with various tags is a pretty standard thing in git.  The rest of the commit message text should describe the change, and it is reasonable to reference another Change-Id in that text.

Would turning on the 'Require a Change-Id' option for your project in Gerrit solve this issue for your team?  That way a commit would be rejected during upload if it didn't have a valid Change-Id tag.
Feb 7, 2011
#7 wal...@google.com
git itself actually has no notion of footers - they are only a convention implemented among humans on top of git. Human conventions tend to be flexible, and not break just because a newline was inserted at the wrong place. Saying 'git does it already!' to justify current gerrit footers behavior seems fallacious to me.
Feb 7, 2011
Project Member #8 bklarson@gmail.com
I respectfully disagree - see the git commit -s argument.  Also jgit has an API for pulling out the commit footer and parsing the tags (http://lists-archives.org/git/694524-add-parsing-support-for-signed-off-by-lines-in-commit-messages.html).  IMO, a commit footer is just as much a git convention as the commit header.
May 15, 2011
#9 sop@google.com
 Issue 663  has been merged into this issue.
May 16, 2011
#10 di...@google.com
Since there is a split group of people that want the feature and people that don't, then make it configurable? In my use case it is much less of an issue for a Change-Id to be mistakenly quoted in the middle of the message than for Gerrit to ignore an intended Change-Id somewhere not in the last paragraph. It has become less important since we enabled to require Change-Id to all reviews but until that happened it has produced pain for more than an year.
Nov 24, 2011
#12 ehun...@broadcom.com
I don't really care where the Change-Id and Signed-off-by tags need to be, but the error message returned when rejecting due to incorrect placement *MUST* indicate where they need to be - It took me quite a while to figure out what was required...
Nov 24, 2011
Project Member #13 mf...@codeaurora.org
ehunter: What version of Gerrit are you using?  If you are using 2.2.1 or later and you still don't like the error messages for this, please suggest an improvement.
Nov 24, 2011
#14 ehun...@broadcom.com
Hi mfick, I'm using version 2.2.1 (on http://openocd.zylin.com) 

The error messages given were: 
! [remote rejected] HEAD -> refs/for/master (not Signed-off-by author/committer/uploader)

or

! [remote rejected] HEAD -> refs/for/master (missing Change-Id in commit message )

Both messages are incorrect - I did have Change-Id and Signed-off-by, but they were not in the correct position. The error message should be something like :

! [remote rejected] HEAD -> refs/for/master (not Signed-off-by author/committer/uploader in last paragraph of commit message)
! [remote rejected] HEAD -> refs/for/master (Change-Id not found in last paragraph of commit message )

Also, it would be nice if it said which commit was being rejected (where there are multiple commits being pushed)

Nov 28, 2011
Project Member #15 mf...@codeaurora.org
I am fairly certain that those error messages where improved quite a bit back in May, I thought in 2.2.1, but I guess not.  Perhaps they are still in master unreleased (there are well over 300 unreleased changes I think there!).  It would probably be worth your while to check out the head.
May 3, 2012
#16 nthieb...@gmail.com
the hick is that git-am -s add a new line after the ChangeId in an 'applied' patch which cause troubles as the commit hook then try to add a new ChangeId.
Jan 9, 2013
Project Member #17 bklarson@gmail.com
 Issue 663  has been merged into this issue.
Sign in to add a comment

Powered by Google Project Hosting