threads / discuss / 32062

Reviews on mailing-list

Subject: Reviews on mailing-list

## tl;dr

17 messages between Nov 10, 2012 and Nov 13, 2012.

replies: 16people: 9as markdown or json

Deniz Türkoglu· Nov 10, 2012, 23:19 UTC · lore
Hi all,

This is my first mail to the git mailing list. I have been following the list for some time now and I would like to suggest moving the reviews out of the mailing list, for example to a gerrit instance, I believe it would improve the commits and the mailing list. I have a filter on 'PATCH', but I feel I miss some of the discussion, and things that I would be interested in.

I have spoken to Shawn Pearce (gerrit project lead, google) and he said he is OK with hosting the gerrit instance.

I would like to hear your thoughts on this.

cheers, -deniz

Felipe Contreras· Nov 10, 2012, 23:40 UTC · re: Deniz Türkoglu · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
Show 11 quoted lines
> This is my first mail to the git mailing list. I have been following
> the list for some time now and I would like to suggest moving the
> reviews out of the mailing list, for example to a gerrit instance, I
> believe it would improve the commits and the mailing list. I have a
> filter on 'PATCH', but I feel I miss some of the discussion, and
> things that I would be interested in.
>
> I have spoken to Shawn Pearce (gerrit project lead, google) and he
> said he is OK with hosting the gerrit instance.
>
> I would like to hear your thoughts on this.

Personally I think reviews on the mailing list is far superior than any other review methods. I've even blogged about it and all the reasons[1]. Gerrit is better than bugzilla, but it still requires a web browser, and logging in.

I love to be able to just hit 'reply' with my favorite MUA, comment inline, and hit send.

Cheers.
[1] http://felipec.wordpress.com/2010/01/19/why-bugzilla-sucks-for-handling-patches/
-- 
Felipe Contreras
Thiago Farina· Nov 11, 2012, 01:13 UTC · re: Felipe Contreras · lore

Re: Reviews on mailing-list

On Sat, Nov 10, 2012 at 9:40 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 19 quoted lines
> On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
>
>> This is my first mail to the git mailing list. I have been following
>> the list for some time now and I would like to suggest moving the
>> reviews out of the mailing list, for example to a gerrit instance, I
>> believe it would improve the commits and the mailing list. I have a
>> filter on 'PATCH', but I feel I miss some of the discussion, and
>> things that I would be interested in.
>>
>> I have spoken to Shawn Pearce (gerrit project lead, google) and he
>> said he is OK with hosting the gerrit instance.
>>
>> I would like to hear your thoughts on this.
>
> Personally I think reviews on the mailing list is far superior than
> any other review methods. I've even blogged about it and all the
> reasons[1]. Gerrit is better than bugzilla, but it still requires a
> web browser, and logging in.
>

Requiring a web browser is a huge requirement, ham?? How come that can be an impediment to move forward way of this awkward way of reviewing patches through email? Switching to Gerrit would mean everyone would be using the same tool instead of anyone using its own email client (gmail, mutt, thunderbird, whatever...) and having to figure out git format-patch, git send-email (--reply-to where?).

There are a lot of issues of having to use email for reviewing patches that I think Gerrit is a superior alternative.

And many people are arguing for it!
Let's move on...
Junio C Hamano· Nov 11, 2012, 05:54 UTC · re: Thiago Farina · lore

Re: Reviews on mailing-list

Thiago Farina <tfransosi@gmail.com> wrote:
>Requiring a web browser is a huge requirement, ham??
No, but requiring reviews and discussions typed in the browser is.
Pardon terseness, typo and HTML from a tablet.
suvayu ali· Nov 11, 2012, 11:11 UTC · re: Thiago Farina · lore

Re: Reviews on mailing-list

Hi,
On Sun, Nov 11, 2012 at 2:13 AM, Thiago Farina <tfransosi@gmail.com> wrote:
Show 34 quoted lines
> On Sat, Nov 10, 2012 at 9:40 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
>>
>>> This is my first mail to the git mailing list. I have been following
>>> the list for some time now and I would like to suggest moving the
>>> reviews out of the mailing list, for example to a gerrit instance, I
>>> believe it would improve the commits and the mailing list. I have a
>>> filter on 'PATCH', but I feel I miss some of the discussion, and
>>> things that I would be interested in.
>>>
>>> I have spoken to Shawn Pearce (gerrit project lead, google) and he
>>> said he is OK with hosting the gerrit instance.
>>>
>>> I would like to hear your thoughts on this.
>>
>> Personally I think reviews on the mailing list is far superior than
>> any other review methods. I've even blogged about it and all the
>> reasons[1]. Gerrit is better than bugzilla, but it still requires a
>> web browser, and logging in.
>>
> Requiring a web browser is a huge requirement, ham?? How come that can
> be an impediment to move forward way of this awkward way of reviewing
> patches through email? Switching to Gerrit would mean everyone would
> be using the same tool instead of anyone using its own email client
> (gmail, mutt, thunderbird, whatever...) and having to figure out git
> format-patch, git send-email (--reply-to where?).
>
> There are a lot of issues of having to use email for reviewing patches
> that I think Gerrit is a superior alternative.
>
> And many people are arguing for it!
>
> Let's move on...

I'm just a user, I found this discussion intriguing and was wondering if any of you have heard of patchwork server[1]. It is a patch aggregator for mailing lists and provides a convenient bug tracker like web interface without getting in the way of the workflow of reviewing patches on the mailing list. If you are interested the Org mode community (an Emacs library) uses it. You can take a look here:

  <http://patchwork.newartisans.com/project/org-mode/list/>
I just thought this might be a nice middle ground for people.
Cheers,
Footnotes:
[1] <http://jk.ozlabs.org/projects/patchwork/>

-- Suvayu

Open source is the future. It sets us free.
Nguyen Thai Ngoc Duy· Nov 11, 2012, 12:02 UTC · re: suvayu ali · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 6:11 PM, suvayu ali <fatkasuvayu+linux@gmail.com> wrote:
Show 10 quoted lines
> I'm just a user, I found this discussion intriguing and was wondering if
> any of you have heard of patchwork server[1].  It is a patch aggregator
> for mailing lists and provides a convenient bug tracker like web
> interface without getting in the way of the workflow of reviewing
> patches on the mailing list.  If you are interested the Org mode
> community (an Emacs library) uses it.  You can take a look here:
>
>   <http://patchwork.newartisans.com/project/org-mode/list/>
>
> I just thought this might be a nice middle ground for people.

It's been brought up several times [1] before. And Shawn wanted to do something about this too [2]. I don't monitor gerrit so I don't know if it has become true.

[1] http://search.gmane.org/search.php?group=gmane.comp.version-control.git&query=patchwork [2] http://thread.gmane.org/gmane.comp.version-control.git/102887/focus=102901

-- 
Duy
Felipe Contreras· Nov 11, 2012, 12:14 UTC · re: Thiago Farina · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 2:13 AM, Thiago Farina <tfransosi@gmail.com> wrote:
> On Sat, Nov 10, 2012 at 9:40 PM, Felipe Contreras
Show 6 quoted lines
>> Personally I think reviews on the mailing list is far superior than
>> any other review methods. I've even blogged about it and all the
>> reasons[1]. Gerrit is better than bugzilla, but it still requires a
>> web browser, and logging in.
>>
> Requiring a web browser is a huge requirement, ham??

Yes. Today people can use any mail interface: web, console-based, graphical. They can use Gmail clients in their phone, or IMAP, or whatever.

Requiring everyone to use a web browser would limit the amount of ways people can review patches. Also, not everyone has javascript enabled in their browser (I assume Gerrit needs that).

> How come that can
> be an impediment to move forward way of this awkward way of reviewing
> patches through email?
It's not awkward, it's the most sensible way.
You just replied to my mail the same way I would reply to a patch.
> Switching to Gerrit would mean everyone would
> be using the same tool instead of anyone using its own email client
> (gmail, mutt, thunderbird, whatever...)
Yes, that's bad.
> and having to figure out git
> format-patch, git send-email (--reply-to where?).
No need to figure anything.

% git config sendemail.to git@vger.kernel.org % git send-email @{upstream}..

Done.
> There are a lot of issues of having to use email for reviewing patches
> that I think Gerrit is a superior alternative.

There are no issues. It works for Linux, qemu, libav, ffmpeg, git, and many other projects.

> And many people are arguing for it!
Nope, they are not.
-- 
Felipe Contreras
Thiago Farina· Nov 11, 2012, 13:09 UTC · re: Felipe Contreras · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 10:14 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

> Requiring everyone to use a web browser would limit the amount of ways
> people can review patches.

I don't see that as a limitation as I think everyone has access to a web browser these days, don't have?

Show 6 quoted lines
>> How come that can
>> be an impediment to move forward way of this awkward way of reviewing
>> patches through email?
>
> It's not awkward, it's the most sensible way.
>
The most harder way I think?

Look at this: https://gerrit.chromium.org/gerrit/#/q/status:open+project:chromiumos/platform/power_manager,n,z

There I can go and see many informations that through this mailing list I can't or have to do much more work in order to archive this.

If you open one of the 'patches' you can see some relevant information:
- Who is the owner/author
- Was it verified?
- Is it ready for landing?
- If I click on Side-by-side I get a nice diff view interface that
plan text email does NOT give me.
- Was it reviewed/approved (+1, +2)?
- It can be merged by one click.
- The interface also provide the command line to download/apply the
patch for me.
- Isn't there a reason (implicit there) for Google being using tools
like Gerrit/CodeReview(rietveld)/Mondrian for handling his code
reviews rather than solely by 'email'?
> You just replied to my mail the same way I would reply to a patch.
>
I replied through a web browser by the Gmail interface. ;)
Show 10 quoted lines
>> There are a lot of issues of having to use email for reviewing patches
>> that I think Gerrit is a superior alternative.
>
> There are no issues. It works for Linux, qemu, libav, ffmpeg, git, and
> many other projects.
>
>> And many people are arguing for it!
>
> Nope, they are not.
>

If they weren't then nobody would be suggesting to use Gerrit for handling the review of git patches.

But I think the big resistance comes from the fact that the core developers handle/review the git patches through Gnus/Emacs, so that is enough for them and they don't want to make the switch because of that?

Felipe Contreras· Nov 11, 2012, 13:40 UTC · re: Thiago Farina · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 2:09 PM, Thiago Farina <tfransosi@gmail.com> wrote:
Show 20 quoted lines
> On Sun, Nov 11, 2012 at 10:14 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Requiring everyone to use a web browser would limit the amount of ways
>> people can review patches.
> I don't see that as a limitation as I think everyone has access to a
> web browser these days, don't have?
>
>>> How come that can
>>> be an impediment to move forward way of this awkward way of reviewing
>>> patches through email?
>>
>> It's not awkward, it's the most sensible way.
>>
> The most harder way I think?
>
> Look at this:
> https://gerrit.chromium.org/gerrit/#/q/status:open+project:chromiumos/platform/power_manager,n,z
>
> There I can go and see many informations that through this mailing
> list I can't or have to do much more work in order to archive this.
That information has nothing to do with reviews. That's patch state-tracking.
> If you open one of the 'patches' you can see some relevant information:
> - Who is the owner/author
> - Was it verified?
> - Is it ready for landing?
Irrelevant for git.
> - If I click on Side-by-side I get a nice diff view interface that
> plan text email does NOT give me.
Not useful.
> - Was it reviewed/approved (+1, +2)?
You can see the same in a mail thread.
> - It can be merged by one click.
Irrelevant for git.
> - The interface also provide the command line to download/apply the
> patch for me.
Not useful.
> - Isn't there a reason (implicit there) for Google being using tools
> like Gerrit/CodeReview(rietveld)/Mondrian for handling his code
> reviews rather than solely by 'email'?
Who knows And if there is, who knows if it's valid.
And none of those points has anything to do with code *review*.

All these points are about state-tracking, and that can be implemented *on top* of the mailing list, for example through patchwork:

http://patchwork.newartisans.com/patch/1531/

That's if somebody actually cared about that, but that doesn't seem to be the case.

>> You just replied to my mail the same way I would reply to a patch.
>>
> I replied through a web browser by the Gmail interface. ;)
Indeed, Gmail is one of the many ways you can review a patch.

You clik reply, you add the comments in line, and click send. Couldn't be easier.

Show 12 quoted lines
>>> There are a lot of issues of having to use email for reviewing patches
>>> that I think Gerrit is a superior alternative.
>>
>> There are no issues. It works for Linux, qemu, libav, ffmpeg, git, and
>> many other projects.
>>
>>> And many people are arguing for it!
>>
>> Nope, they are not.
>>
> If they weren't then nobody would be suggesting to use Gerrit for
> handling the review of git patches.
Except you, of course.
> But I think the big resistance comes from the fact that the core
> developers handle/review the git patches through Gnus/Emacs, so that
> is enough for them and they don't want to make the switch because of
> that?

gnus/emacs/notmuch/thunderbird/Gmail, and pretty much every mail client out there.

Cheers.
-- 
Felipe Contreras
Krzysztof Mazur· Nov 11, 2012, 17:14 UTC · re: Thiago Farina · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 11:09:36AM -0200, Thiago Farina wrote:
Show 6 quoted lines
> On Sun, Nov 11, 2012 at 10:14 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > Requiring everyone to use a web browser would limit the amount of ways
> > people can review patches.
> I don't see that as a limitation as I think everyone has access to a
> web browser these days, don't have?
Almost everyone, but not everyone likes using web browser.

With emails you can use your favorite interface: favorite MUA and in good MUA also your favorite editor (program for writing text, not some editor-like product in web browser). With web-based system you have only ONE interface (or few), not chosen by you, you cannot change it, web admin may change it in way you not like.

Krzysiek
David Lang· Nov 11, 2012, 21:15 UTC · re: Krzysztof Mazur · lore

Re: Reviews on mailing-list

On Sun, 11 Nov 2012, Krzysztof Mazur wrote:
Show 9 quoted lines
> On Sun, Nov 11, 2012 at 11:09:36AM -0200, Thiago Farina wrote:
>> On Sun, Nov 11, 2012 at 10:14 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> Requiring everyone to use a web browser would limit the amount of ways
>>> people can review patches.
>> I don't see that as a limitation as I think everyone has access to a
>> web browser these days, don't have?
>
> Almost everyone, but not everyone likes using web browser.
Using a web browser requires connectivity at the time you are doing the review.

Mailing list based reviews can be done at times when you don't have connectivity.

Coincidently, one of the common times when people don't have connectivity is when they are traveling, and this can be a good time to do things like code review.

David Lang
Deniz Türkoglu· Nov 12, 2012, 00:35 UTC · re: David Lang · lore

Re: Reviews on mailing-list

I understand from the feedback that gerrit should get better on making it possible to review code via e-mail, as pointed out in Nguyen's mail, a flow like Shawn mentioned[1] can be a good solution.

FWIW, I can fetch the change(s) from gerrit I am interested in and review it any time I want. I currently have many checked out topics I am working on for instance.

[1] http://thread.gmane.org/gmane.comp.version-control.git/102887/focus=102901

cheers, -deniz

David Lang· Nov 12, 2012, 00:43 UTC · re: Deniz Türkoglu · lore

Re: Reviews on mailing-list

On Sun, 11 Nov 2012, Deniz Türkoglu wrote:
Show 7 quoted lines
> I understand from the feedback that gerrit should get better on making
> it possible to review code via e-mail, as pointed out in Nguyen's
> mail, a flow like Shawn mentioned[1] can be a good solution.
>
> FWIW, I can fetch the change(s) from gerrit I am interested in and
> review it any time I want. I currently have many checked out topics I
> am working on for instance.

That requires that you know before you loose connectivity what changes you want to review.

With e-mail based reviews, you just pull copies of all your mail and it includes any pending reviews along with everything else.

David Lang
Nguyen Thai Ngoc Duy· Nov 13, 2012, 13:38 UTC · re: David Lang · lore

Re: Reviews on mailing-list

On Mon, Nov 12, 2012 at 4:15 AM, David Lang <david@lang.hm> wrote:
Show 5 quoted lines
> Using a web browser requires connectivity at the time you are doing the
> review.
>
> Mailing list based reviews can be done at times when you don't have
> connectivity.

I am not against email-based reviews but I'd like to point out that with Google Gears (and HTML5 Storage?) Gerrit can be made work offline too. I don't know how much work required though.

-- 
Duy
Deniz Türkoglu· Nov 11, 2012, 01:28 UTC · re: Felipe Contreras · lore

Re: Reviews on mailing-list

On Sat, Nov 10, 2012 at 3:40 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 18 quoted lines
> On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
>
>> This is my first mail to the git mailing list. I have been following
>> the list for some time now and I would like to suggest moving the
>> reviews out of the mailing list, for example to a gerrit instance, I
>> believe it would improve the commits and the mailing list. I have a
>> filter on 'PATCH', but I feel I miss some of the discussion, and
>> things that I would be interested in.
>>
>> I have spoken to Shawn Pearce (gerrit project lead, google) and he
>> said he is OK with hosting the gerrit instance.
>>
>> I would like to hear your thoughts on this.
>
> Personally I think reviews on the mailing list is far superior than
> any other review methods. I've even blogged about it and all the
> reasons[1]. Gerrit is better than bugzilla, but it still requires a
> web browser, and logging in.

I disagree that the current approach is optimal. Bugzilla is a bug-tracker and is not meant to be used for reviews. I believe in using the right tool for the right job. An e-mail should be concise and to the point, in this case only contain the discussion. This will help it to reach a wider audience and be more useful when people stumble upon it through a google search.

cheers, -deniz

Felipe Contreras· Nov 11, 2012, 12:24 UTC · re: Deniz Türkoglu · lore

Re: Reviews on mailing-list

On Sun, Nov 11, 2012 at 2:28 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
Show 27 quoted lines
> On Sat, Nov 10, 2012 at 3:40 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sun, Nov 11, 2012 at 12:19 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
>>
>>> This is my first mail to the git mailing list. I have been following
>>> the list for some time now and I would like to suggest moving the
>>> reviews out of the mailing list, for example to a gerrit instance, I
>>> believe it would improve the commits and the mailing list. I have a
>>> filter on 'PATCH', but I feel I miss some of the discussion, and
>>> things that I would be interested in.
>>>
>>> I have spoken to Shawn Pearce (gerrit project lead, google) and he
>>> said he is OK with hosting the gerrit instance.
>>>
>>> I would like to hear your thoughts on this.
>>
>> Personally I think reviews on the mailing list is far superior than
>> any other review methods. I've even blogged about it and all the
>> reasons[1]. Gerrit is better than bugzilla, but it still requires a
>> web browser, and logging in.
>
> I disagree that the current approach is optimal. Bugzilla is a
> bug-tracker and is not meant to be used for reviews. I believe in
> using the right tool for the right job. An e-mail should be concise
> and to the point, in this case only contain the discussion. This will
> help it to reach a wider audience and be more useful when people
> stumble upon it through a google search.

I don't understand what you are saying. If you google 'git reviews on mailing lit', you will find results like this:

http://article.gmane.org/gmane.comp.version-control.git/209313

You don't get any patches because you didn't search for patches, and either way Google would not filter out the results from gerrit either. For example: googing 'cyanogenmod "Remove tabs from GNexusParts" will throw:

http://review.cyanogenmod.org/
-- 
Felipe Contreras
Ramkumar Ramachandra· Nov 11, 2012, 07:26 UTC · re: Deniz Türkoglu · lore

Re: Reviews on mailing-list

Deniz Türkoglu wrote:
> I have spoken to Shawn Pearce (gerrit project lead, google) and he
> said he is OK with hosting the gerrit instance.
>
> I would like to hear your thoughts on this.
I personally think email is by far the best interface for patches,
reviews, and discussions.  Git patches are very high-volume, and not
everyone can read everything.  People should have the flexibility to
choose the client they'd like to use to read patches and follow-ups;
the freedom to use a scriptable client like Gnus is very important to
me. Primarily, I want people to be able to:
1. Choose what to read, by scripting Gnus to score email that they'd
likely find relevant.
2. Try out new patches on the list, by assigning one keybinding to
git-am a series.
3. Display email the way they like.  Many email clients have features
to run filters through emails.
4. Read patches/ follow-ups offline, while travelling (on a phone, for
instance).  The GMail app, for instance, downloads mails for offline
viewing.
5. Interact with other lists seamlessly (the kernel list, for
instance).  Email is a universal interface on which lists can be CC'ed
easily.

I'm not attacking a specific web interface, but I don't see how any of the following would be possible even with the most advanced web interface. Besides, nobody has made a proper case for using one. Therefore, I'm strongly opposed to the move.

Ram

← back to recent threads