Gerrit notes in git log

637 views
Skip to first unread message

Duncan Laurie

unread,
Jun 23, 2011, 4:07:17 PM6/23/11
to Chromium OS dev
Gerrit exports information about a given Change-Id review that can be
pulled into your client and displayed as part of the log message.
This does not include review comments but does include some other
(potentially) useful information.

It is easy to enable, for example from within the kernel repository:

$ git config --add remote.cros.fetch +refs/notes/*:refs/notes/*
$ git config notes.displayRef refs/notes/*
$ git remote update

And then look at a commit and you should see another section in the log:
$ git show dff00d1

commit dff00d1da70e860d50f9b361ce5acf6eb197f1c1
Author: Mandeep Singh Baines <m...@chromium.org>
Date: Mon Jun 20 15:30:12 2011 -0700

CHROMIUM: preserved: use wbinvd instead of set_memory_uc().

set_memory_uc() does a bunch of crazy stuff that is probably not
safe in the context of panic.

BUG=chromium-os:14497
TEST=platform_KernelErrorPaths

Change-Id: Ie41e3213b22456df9e13a179066b85fd3ff2f717
Signed-off-by: Mandeep Singh Baines <m...@chromium.org>
Reviewed-on: http://gerrit.chromium.org/gerrit/2908
Reviewed-by: Duncan Laurie <dla...@chromium.org>
Reviewed-by: Simon Glass <s...@chromium.org>

Notes (review):
Verified+1: Mandeep Singh Baines <m...@chromium.org>
Code-Review+2: Duncan Laurie <dla...@chromium.org>
Code-Review+1: Simon Glass <s...@chromium.org>
Submitted-by: Mandeep Singh Baines <m...@chromium.org>
Submitted-at: Tue, 21 Jun 2011 10:15:12 -0700
Reviewed-on: http://gerrit.chromium.org/gerrit/2908
Project: chromiumos/third_party/kernel
Branch: refs/heads/chromeos-2.6.38


Hopefully this will be useful to others.

-duncan

Rich Schmitt

unread,
Jan 13, 2013, 10:55:16 AM1/13/13
to chromiu...@chromium.org
This is very useful.  On a related topic. 

Is there a reason why gerrit puts the change-id in the commit-msg rather then in the notes.  For that matter, the minor bugzilla integration also expects the bug-id to be noted in the commit-msg.  We do not want to preserve this process specific information when the patch is ultimately pulled upstream.   In order to preserve commit SHAs when upstreaming, we'd rather gerrit leverage this information in a git note.

I believe it is straight forward enough to do and we are prepared to take it on, unless of course there has been some work on this already.

Any ideas?

Rich

Brian Harring

unread,
Jan 14, 2013, 2:50:46 AM1/14/13
to Rich Schmitt, Chromium OS dev
On Sun, Jan 13, 2013 at 7:55 AM, Rich Schmitt <blue6...@gmail.com> wrote:
This is very useful.  On a related topic. 

Is there a reason why gerrit puts the change-id in the commit-msg rather then in the notes.  For that matter, the minor bugzilla integration also expects the bug-id to be noted in the commit-msg.  We do not want to preserve this process specific information when the patch is ultimately pulled upstream.   In order to preserve commit SHAs when upstreaming, we'd rather gerrit leverage this information in a git note.

I believe it is straight forward enough to do and we are prepared to take it on, unless of course there has been some work on this already.

Commit messages carry the Change-Id footer since it's the only way gerrit knows how to route a given revision- whether to open a new review/change page, or to add that new rev as a patchset on an existing review/change.  The full docs are https://gerrit.chromium.org/gerrit/Documentation/user-changeid.html .

Pretty much, there isn't a sane way to pull that out- any attempt to pull the footer out of the commit message is just going to make users lives hell since they would have to manage it themselves.

The problem here is that git commit messages using git footers to pass metadata are a normal enough approach- signed-off-by, acked-by, etc.  What you're asking gerrit to do is to strip our footers out of each/every change we have (thus changing the sha1 of our own patchset series) and shoving it into the notes namespace.  That's not a simple change, and that *only* would work for refs/for/* namespace- aka, gerrit review CLs.  Any direct pushes, any merges, etc, would have to be rewritten on the fly doing the same thing.

That's a fairly disruptive change for anyone using gerrit frankly, and unlikely to be exactly an easy sell.

Personally, I'd prefer if gerrit did not embed review information into the commit message- it does it only in certain cases, not in all.  Since that info is available via notes, grabbing it from there is my preference- that said I don't think the majority of our users make use of refs/notes/review namespace, instead using the commit message, so hacking gerrit to stop injecting that info into the commit message is unlikely to be popular.

Any ideas?
 
Offhand, frankly I'd just leave the commit messages alone- I assume y'all don't scrape out signed-off-by's from contributors, this mostly is no different (the difference being we don't use ':' for delineation so they're not proper footers, although w/ enough yelling that is potentially changable).

Perhaps you can clarify the why for this?  Technically if it's a large issue, you can just filter-branch the messages to what you want- at the cost of making interop between our branches and y'alls a bit of a PITA however.  I really do not advise doing that however. :)

~harring
Reply all
Reply to author
Forward
This conversation is locked
You cannot reply and perform actions on locked conversations.
0 new messages