Fwd: git-reviewed: linking commits to review discussion in git

Fwd: git-reviewed: linking commits to review discussion in git

[Murtuza Mukadam]

Hello All,

We have linked peer review discussions on
git@vger.kernel.org to their respective commits within the main
git.git repository. You can view the linked reviews from 2012
until present in the GitHub repo at:
https://github.com/mmukadam/git/tree/review

If you want to search for the reviews related to a commit, you can
type in the full hash code here:
http://users.encs.concordia.ca/~m_mukada/git-review-tracker.html

or append the commit hash to the following url:
https://github.com/mmukadam/git/tree/review/<commit_hash>

Note, if you get a '404 page not found' from GitHub it means that we
couldn't find a review to link the commit to.

It's a bit hard to view the reviews on GitHub because the 'review' branch was
designed to be viewed on the command line using a set of simple scripts that
sit on top of the standard git commands.

For example:
to view all reviews Hamano is involved in you can do:
git reviewed --log-reviewer hamano

to view the reviews of particular commit:
git reviewed --log 1a1832eb085e5bca198735e5d0e766a3cb61b8fc

You can also create new reviews on a commit and mail them as well as
import reviews from email.

Note the command is git-reivewED because git-review is associated with Gerrit.

The tool is beta, but it just creates a detached review branch, so it
won't affect master. To remove it all you do is delete the scripts and
the 'review' branch. More info available here:

The online man-page is here:
http://users.encs.concordia.ca/~m_mukada/git-review.html

In order to install git-review, you need to clone the repository:
https://github.com/mmukadam/git-reviewed.git

The online tutorial is available here:
http://users.encs.concordia.ca/~m_mukada/git-review-tutorial.html


Sample output:

Let's say you want to see the reviews related to commit:
---------------------------------------------------------------------------------
git reviewed --log 9ce415d972a04df4bfceaad0fab8eaea9a66997e

Review: 74d203accc341993dbed507fceceb2841670ce37
Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Author:     Johannes Sixt <j6t@kdbg.org>
AuthorDate: Fri Jun 7 22:53:27 2013 +0200
Reviewer:   Junio C Hamano gitster@pobox.com <gitster@pobox.com>
ReviewDate: Thu Feb 13 11:35:13 2014 -0800


    Re: [PATCH] tests: turn on network daemon tests by default

Review: e80ebaa1fdd132a13bfbc6f87cdd4ecec7f2b715
Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Author:     Johannes Sixt <j6t@kdbg.org>
AuthorDate: Fri Jun 7 22:53:27 2013 +0200
Reviewer:   Junio C Hamano gitster@pobox.com <gitster@pobox.com>
ReviewDate: Wed Feb 12 14:34:52 2014 -0800


    Re: [PATCH] tests: turn on network daemon tests by default

Review: 8da03766afcd60e432ffdd2d6f549e2ed02a4bc9
Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Author:     Johannes Sixt <j6t@kdbg.org>
AuthorDate: Fri Jun 7 22:53:27 2013 +0200
Reviewer:   Jeff King peff@peff.net <peff@peff.net>
ReviewDate: Mon Feb 10 16:29:37 2014 -0500


    [PATCH] tests: turn on network daemon tests by default



To show the email review discussion for a single review:
---------------------------------------------------------------------------------

git reviewed --view 74d203accc341993dbed507fceceb2841670ce37

Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Reviewer: Junio C Hamano <gitster@pobox.com>
Reviewer Date: Thu, 13 Feb 2014 11:35:13 -0800
---------------------------------------------------------------
From : Junio C Hamano <gitster@pobox.com>
Message-Id : <xmqqfvnmlsb2.fsf@gitster.dls.corp.google.com>
Date : Thu, 13 Feb 2014 11:35:13 -0800
Subject: Re: [PATCH] tests: turn on network daemon tests by default
References: <20140210191521.GA3112@sigill.intra.peff.net>
<20140210212931.GA16154@sigill.intra.peff.net>
<xmqqa9dxpgw9.fsf@gitster.dls.corp.google.com>
<20140211200445.GA27946@sigill.intra.peff.net>
<xmqqzjlxnqvw.fsf@gitster.dls.corp.google.com>
<20140212214753.GA6799@sigill.intra.peff.net>
<xmqq38jom037.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqq38jom037.fsf@gitster.dls.corp.google.com> (Junio C.
Hamano's message of "Wed, 12 Feb 2014 14:34:52 -0800")
Cc: Duy Nguyen <pclouds@gmail.com>, Git Mailing List
<git@vger.kernel.org>, "Shawn O. Pearce" <spearce@spearce.org>
commit: 9ce415d972a04df4bfceaad0fab8eaea9a66997e

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>>   test_normalize_tristate GIT_TEST_DAEMON
>
> Heh, great minds think alike.  This is what I am playing with,
> without committing (because I do like your "ask config if this is a
> kind of various boolean 'false' representations, which I haven't
> managed to add to it).

[snip]

---
 t/lib-git-daemon.sh     |  8 ++++---
 t/lib-httpd.sh          | 22 +++++++++----------
 t/test-lib-functions.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 14 deletions(-)

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index 394b06b..615bf5d 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -16,9 +16,10 @@
 #      stop_git_daemon
 #      test_done

-if test -z "$GIT_TEST_GIT_DAEMON"
+test_tristate GIT_TEST_GIT_DAEMON
+if test "$GIT_TEST_GIT_DAEMON" = false
 then
-       skip_all="git-daemon testing disabled (define
GIT_TEST_GIT_DAEMON to enable)"
+       skip_all="git-daemon testing disabled (unset
GIT_TEST_GIT_DAEMON to enable)"
        test_done
 fi

[snip]

 # The following mingw_* functions obey POSIX shell syntax, but are actually
 # bash scripts, and are meant to be used only with bash on Windows.

--
1.9.0-rc3-260-g4cf525c


To see all the reviews anyone is involved in:
-------------------------------------------------------------------

git reviewed --log reviewer hamano

Review: ecb6f98955700169ed6f88d929babd7d0f770116
Commit Reviewed: 9c8ce7397bac108f83d77dfd96786edb28937511
Author:     Michael J Gruber <git@drmicha.warpmail.net>
AuthorDate: Thu Feb 13 11:35:08 2014 +0100
Reviewer:   Junio C Hamano gitster@pobox.com <gitster@pobox.com>
ReviewDate: Thu Feb 13 13:25:23 2014 -0800


    Re: [PATCH] release notes: typo fixes

Review: 74d203accc341993dbed507fceceb2841670ce37
Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Author:     Johannes Sixt <j6t@kdbg.org>
AuthorDate: Fri Jun 7 22:53:27 2013 +0200
Reviewer:   Junio C Hamano gitster@pobox.com <gitster@pobox.com>
ReviewDate: Thu Feb 13 11:35:13 2014 -0800


    Re: [PATCH] tests: turn on network daemon tests by default

Review: e80ebaa1fdd132a13bfbc6f87cdd4ecec7f2b715
Commit Reviewed: 9ce415d972a04df4bfceaad0fab8eaea9a66997e
Author:     Johannes Sixt <j6t@kdbg.org>
AuthorDate: Fri Jun 7 22:53:27 2013 +0200
Reviewer:   Junio C Hamano gitster@pobox.com <gitster@pobox.com>
ReviewDate: Wed Feb 12 14:34:52 2014 -0800


    Re: [PATCH] tests: turn on network daemon tests by default



Please let us know what you think about this tool as it will help us
in improving it. If you'd like us to extract the reviews related to a
different repository, please let us know.

This is part of my Msc thesis, so any feedback is appreciated.
We thank you for you time.

Cheers,
Murtuza

Re: Fwd: git-reviewed: linking commits to review discussion in git

[Jeff King]

On Mon, Feb 17, 2014 at 03:12:48PM -0500, Murtuza Mukadam wrote:

> We have linked peer review discussions on
> git@vger.kernel.org to their respective commits within the main
> git.git repository. You can view the linked reviews from 2012
> until present in the GitHub repo at:
> https://github.com/mmukadam/git/tree/review

Neat. We've experimented in the past with mapping commits back to
mailing list discussions.  Thomas (cc'd) has a script that creates
git-notes trees mapping commits to the relevant message-id, which can
then be found in the list archive.

To me, the interesting bits of such a project are:

  1. How do we decide which messages led to which commits? There is
     definitely some room for heuristics here, as patches are sometimes
     tweaked in transit, or come in multiple stages (e.g., the original
     patch, then somebody suggests a fixup on top). You might want to
     compare your work with the script from Thomas here:

       http://repo.or.cz/w/trackgit.git

  2. How do we store the mapping? I think git-notes are a natural fit
     here, but you don't seem to use them. Is there a reason?

  3. How do we present the emails to the user (including showing
     threads, letting them dig deeper, etc)?

     The existing solution has no support at all for 3. Personally, I
     keep my own git-list archive locally, so I can search it (by
     message-id or other features), dump the result into an mbox
     (optionally including the surrounding thread), and then view the
     result in mutt.

Having had this solution for a while, my experience has been that I
don't use it that often. It's not that I don't refer to the archive to
see more backstory on a commit; I probably do that once a week or so.
But since I have a decent searchable archive, I tend to just do it "by
hand", searching for keywords from the commit message, and limiting by
date if necessary.

Going straight to the message by id might be a little faster, but I
often pick up stray bits in my search that were not part of the original
thread. E.g., somebody reports a bug, then 3 days later, somebody else
posts a patch (but does not do it as a reply to the bug). There's
nothing in the message headers or the commit mapping to say that those
two messages are related. But because a search of the relevant terms
finds both, and because the result is date-sorted, they end up near each
other and it's easy for me to peruse.

It would be interesting to apply some kind of clustering algorithm that
automatically determines the messages related to a commit, including
both the patch but also any discussion leading up to it. I realize that
may be getting far afield of your original goals, but hey, you said you
wanted feedback. I can reach for the stars. :)

-Peff

Re: Fwd: git-reviewed: linking commits to review discussion in git

[Thomas Rast]

Jeff King <peff@peff.net> writes:

> On Mon, Feb 17, 2014 at 03:12:48PM -0500, Murtuza Mukadam wrote:
>
>> We have linked peer review discussions on
>> git@vger.kernel.org to their respective commits within the main
>> git.git repository. You can view the linked reviews from 2012
>> until present in the GitHub repo at:
>> https://github.com/mmukadam/git/tree/review
>
> Neat. We've experimented in the past with mapping commits back to
> mailing list discussions.  Thomas (cc'd) has a script that creates
> git-notes trees mapping commits to the relevant message-id, which can
> then be found in the list archive.
>
> To me, the interesting bits of such a project are:
>
>   1. How do we decide which messages led to which commits? There is
>      definitely some room for heuristics here, as patches are sometimes
>      tweaked in transit, or come in multiple stages (e.g., the original
>      patch, then somebody suggests a fixup on top). You might want to
>      compare your work with the script from Thomas here:
>
>        http://repo.or.cz/w/trackgit.git

Eh, or don't.  My script nowadays uses Junio's suggestion of matching on
(author, authordate) with a little bit of tweaking in case there is no
match.  The name/date match works for most cases even in slightly
tweaked forms.

(The very first version elaborately tried all sorts of things, including
attempting to patch on master, next etc. to see where it applies, and
turned out to be waaaay too slow.)

I'm no longer convinced that there's anything a computer can do beyond
(author, authordate), anyway.  Perhaps someone with a clue in UIs --
that's definitely not me -- could make a website where users can
complete or correct the autogenerated mappings to go further.

-- 
Thomas Rast
tr@thomasrast.ch

Re: Fwd: git-reviewed: linking commits to review discussion in git

[Peter C Rigby]

On Tue, Feb 18, 2014 at 12:28 AM, Jeff King <peff@peff.net> wrote:
[snip]
>
>   3. How do we present the emails to the user (including showing
>      threads, letting them dig deeper, etc)?

We created a website, you enter a commit and it shows you the reviews:
http://cesel.encs.concordia.ca/process_request.php?repo=git

The site will either display the message id for the review or redirect
you the mailing list archive.


On Sat, Feb 22, 2014 at 11:47 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Mon, Feb 17, 2014 at 03:12:48PM -0500, Murtuza Mukadam wrote:

Very sorry for the delayed response. We wanted compare our linking
technique with yours, see below.

>>
>>> We have linked peer review discussions on
>>> git@vger.kernel.org to their respective commits within the main
>>> git.git repository. You can view the linked reviews from 2012
>>> until present in the GitHub repo at:
>>> https://github.com/mmukadam/git/tree/review
>>
>> Neat. We've experimented in the past with mapping commits back to
>> mailing list discussions.  Thomas (cc'd) has a script that creates
>> git-notes trees mapping commits to the relevant message-id, which can
>> then be found in the list archive.
>>
>> To me, the interesting bits of such a project are:
>>
>>   1. How do we decide which messages led to which commits? There is
>>      definitely some room for heuristics here, as patches are sometimes
>>      tweaked in transit, or come in multiple stages (e.g., the original
>>      patch, then somebody suggests a fixup on top). You might want to
>>      compare your work with the script from Thomas here:
>>
>>        http://repo.or.cz/w/trackgit.git
>
> Eh, or don't.  My script nowadays uses Junio's suggestion of matching on
> (author, authordate) with a little bit of tweaking in case there is no
> match.  The name/date match works for most cases even in slightly
> tweaked forms.

In our technique, we take each email patch, eliminate white space and
hash each line. We then compare the lines with those in commits to the
same files. The commit that changes the same files and has the largest
number of matching lines is considered to be the reviewed commit.

We compared Junio's (author, authordate) and our technique on a
manually created benchmark of 30 messages from the linux, postgres,
and git mailing lists. We found that Junio's simple technique
performed equally well to our more complicated technique (see results
at end of email).

On Tue, Feb 18, 2014 at 12:28 AM, Jeff King <peff@peff.net> wrote:
>   2. How do we store the mapping? I think git-notes are a natural fit
>      here, but you don't seem to use them. Is there a reason?

We wanted a way to store each review as its own blob, so we put the
reviews on a detached 'review' branch and then wrote some simple
scripts to access and display these reviews on the command line (\eg
git reviewed --show <commit_hash>).

However, given the previous discussion on this list
(http://git.661346.n2.nabble.com/RFC-RFH-Fun-things-with-git-notes-or-patch-tracking-backwards-td2297330.html),
we agree that just putting the msg_id as a note is probably more
useful. One additional problem we ran into when storing all the
reviews, is for something like Linux, the reviews took up a massive
amount of storage.

> It would be interesting to apply some kind of clustering algorithm that
> automatically determines the messages related to a commit, including
> both the patch but also any discussion leading up to it. I realize that
> may be getting far afield of your original goals, but hey, you said you
> wanted feedback. I can reach for the stars. :)

It would be interesting to be able to tie in other discussion, perhaps
related bugs. A future project.

Thanks,
Peter

Evaluation of linking commits with email reviews on 30 messages from
Linux, postgres, and git mailing lists

Linux - Files and lines changed:

Perfect: 87%
No Match: 13%
Incorrect: 0%

Linux - Thomas/Junio  (author, authordate)

Perfect: 87%
No Match: 13%
Incorrect: 0%

Git - Files and lines changed:

Perfect: 74 %
No Match  23  %
Incorrect: 3 %

Git - Thomas/Junio  (author, authordate)

Perfect: 77%
No Match: 23%
Incorrect: 0%

Postgres - Files and lines changed:

Perfect: 57 %
No Match  36  %
Incorrect: 7 %

Postgres - Thomas/Junio  (author, authordate)

Perfect: 50%
No Match: 37%
Incorrect: 13%

-- 
http://users.encs.concordia.ca/~pcr/