Reviews on mailing-list

Reviews on mailing-list

[Deniz Türkoglu]

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

Re: Reviews on mailing-list

[Felipe Contreras]

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 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

Re: Reviews on mailing-list

[Thiago Farina]

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...

Re: Reviews on mailing-list

[Deniz Türkoglu]

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.

cheers,
-deniz

Re: Reviews on mailing-list

[Junio C Hamano]

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.

Re: Reviews on mailing-list

[Ramkumar Ramachandra]

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

Re: Reviews on mailing-list

[suvayu ali]

Hi,

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
> <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.

Re: Reviews on mailing-list

[Nguyen Thai Ngoc Duy]

On Sun, Nov 11, 2012 at 6:11 PM, suvayu ali <fatkasuvayu+linux@gmail.com> wrote:
> 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

Re: Reviews on mailing-list

[Felipe Contreras]

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

>> 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

Re: Reviews on mailing-list

[Felipe Contreras]

On Sun, Nov 11, 2012 at 2:28 AM, Deniz Türkoglu <deniz@spotify.com> wrote:
> 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

Re: Reviews on mailing-list

[Thiago Farina]

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.

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. ;)

>> 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?

Re: Reviews on mailing-list

[Felipe Contreras]

On Sun, Nov 11, 2012 at 2:09 PM, Thiago Farina <tfransosi@gmail.com> 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?
>
>>> 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.

>>> 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

Re: Reviews on mailing-list

[Krzysztof Mazur]

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.

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

Re: Reviews on mailing-list

[David Lang]

On Sun, 11 Nov 2012, Krzysztof Mazur wrote:

> 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

Re: Reviews on mailing-list

[Deniz Türkoglu]

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

Re: Reviews on mailing-list

[David Lang]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 656 bytes --]

On Sun, 11 Nov 2012, Deniz Türkoglu wrote:

> 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

Re: Reviews on mailing-list

[Nguyen Thai Ngoc Duy]

On Mon, Nov 12, 2012 at 4:15 AM, David Lang <david@lang.hm> wrote:
> 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