Patches review

[Alena Prokharchyk <Al...@citrix.com>]

I know it's been discussed in several email threads, but I would like to
initiate a separate discussion on what tool we should use for reviweing
the patches. 

Several people (including myself - using Outlook on Mac OS X Lion) have
been struggling already with applying email patches using "git am". Some
patches appear to be broken, email file import/save is different in
various email clients, etc. But the main disadvantage - there is no other
way to track patch flow history rather than gathering email by subject.
For instance, I would like to see the patch history in some centralized
place:

* when patch was created
* who picked up the patch for the review and when
* what was fixed after first, second,...n review
* when the patch was merged to the mainstream.

I think we should start using the official tool for that -
Gerrit/Reviewboard/etc.

Please follow up with your suggestions and preferences.

-Alena.

RE: Patches review

[Prachi Damle <Pr...@citrix.com>]

+1 to use some tool for review rather than using emails.

I am struggling with email patches through Outlook still and haven't found a clean solution yet. 
I have no personal experience but have heard about Gerrit being good to achieve what is being listed below by Alena.

-Prachi

-----Original Message-----
From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com] 
Sent: Tuesday, June 12, 2012 12:46 PM
To: cloudstack-dev@incubator.apache.org
Subject: Patches review

I know it's been discussed in several email threads, but I would like to initiate a separate discussion on what tool we should use for reviweing the patches. 

Several people (including myself - using Outlook on Mac OS X Lion) have been struggling already with applying email patches using "git am". Some patches appear to be broken, email file import/save is different in various email clients, etc. But the main disadvantage - there is no other way to track patch flow history rather than gathering email by subject.
For instance, I would like to see the patch history in some centralized
place:

* when patch was created
* who picked up the patch for the review and when
* what was fixed after first, second,...n review
* when the patch was merged to the mainstream.

I think we should start using the official tool for that - Gerrit/Reviewboard/etc.

Please follow up with your suggestions and preferences.

-Alena.

RE: Patches review

[Kevin Kluge <Ke...@citrix.com>]

I think it's unreasonable to ask people to change their mail infrastructure.  Even client change is quite annoying.   So +1 on anything other than e-mail.

I would also add that I've found it difficult to track what has been applied and what hasn't.  Maybe when the maintainer system is fully implemented this will be easier given broader ownership, but if a tool could help us track (as in Alena's suggestions) that would be a big win.   It should also help us be more responsive to the community as it'd help ensure we respond to patches.

-kevin

> -----Original Message-----
> From: David Nalley [mailto:david@gnsa.us]
> Sent: Tuesday, June 12, 2012 1:02 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Patches review
> 
> On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
> <Al...@citrix.com> wrote:
> > I know it's been discussed in several email threads, but I would like
> > to initiate a separate discussion on what tool we should use for
> > reviweing the patches.
> >
> > Several people (including myself - using Outlook on Mac OS X Lion)
> > have been struggling already with applying email patches using "git
> > am". Some patches appear to be broken, email file import/save is
> > different in various email clients, etc. But the main disadvantage -
> > there is no other way to track patch flow history rather than gathering
> email by subject.
> > For instance, I would like to see the patch history in some
> > centralized
> > place:
> >
> > * when patch was created
> > * who picked up the patch for the review and when
> > * what was fixed after first, second,...n review
> > * when the patch was merged to the mainstream.
> >
> > I think we should start using the official tool for that -
> > Gerrit/Reviewboard/etc.
> >
> > Please follow up with your suggestions and preferences.
> >
> > -Alena.
> >
> >
> 
> Thanks for starting this - I have a thrice rewritten mail sitting in my drafts
> folder around this subject. Quick followup to voice some of my frustrations.
> 
> We have a process today - and for a number of folks that has worked very
> well. I personally find it dead easy to grab patches from github (though our
> mirroring is currently non-functional since we have made the move to the
> ASF. ). There's also a certain class of folks that have have sent patches via
> email that were also easy to apply.
> 
> However, a number of folks are sitting behind servers or services that
> actively break patches which has led to much gnashing of teeth here.
> While I don't care so much about MUA issues, I do desperately despise
> seeing MTAs breaking patches - and I spent around 4 hours last night trying
> to unbreak patches from 3 different developers that their MTA (all different)
> had made completely unusable.
> 
> Where this really disturbs me is the barrier to participation. Working on
> CloudStack is a pretty large barrier to begin with. You have to understand the
> facet of CloudStack that you are working on, and then understand git.
> 
> While many of us can work via email - and some of us (me included) even
> prefer it, I do worry about what the net effect is now that we say - git
> patches submitted this way are actively broken by exchange, appear to be
> broken by gmail, etc. I suppose we could push folks to use some external
> mail service that behaves properly, but it seems like an artificial barrier, and
> one that is largely out of the control of folks wishing to collaborate with us.
> 
> I think there are potentially benefits to using some tool other than email
> down the road as well such as being able to have $test_suite run against any
> patch before a committer even gets to review it.
> 
> Thoughts, comments, flames?
> 
> --David

RE: Patches review

[Kelven Yang <ke...@citrix.com>]

A patch management system is definitely very helpful, doing it in both places would be even better. 

Kelven

> -----Original Message-----
> From: Fred Wittekind [mailto:rom@twister.dyndns.org]
> Sent: Wednesday, June 13, 2012 6:00 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Patches review
> 
> On 6/12/2012 4:01 PM, David Nalley wrote:
> > On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
> > <Al...@citrix.com> wrote:
> >> I know it's been discussed in several email threads, but I would like
> to
> >> initiate a separate discussion on what tool we should use for
> reviweing
> >> the patches.
> >>
> >> Several people (including myself - using Outlook on Mac OS X Lion)
> have
> >> been struggling already with applying email patches using "git am".
> Some
> >> patches appear to be broken, email file import/save is different in
> >> various email clients, etc. But the main disadvantage - there is no
> other
> >> way to track patch flow history rather than gathering email by subject.
> >> For instance, I would like to see the patch history in some
> centralized
> >> place:
> >>
> >> * when patch was created
> >> * who picked up the patch for the review and when
> >> * what was fixed after first, second,...n review
> >> * when the patch was merged to the mainstream.
> >>
> >> I think we should start using the official tool for that -
> >> Gerrit/Reviewboard/etc.
> >>
> >> Please follow up with your suggestions and preferences.
> >>
> >> -Alena.
> >>
> >>
> > Thanks for starting this - I have a thrice rewritten mail sitting in
> > my drafts folder around this subject. Quick followup to voice some of
> > my frustrations.
> >
> > We have a process today - and for a number of folks that has worked
> > very well. I personally find it dead easy to grab patches from github
> > (though our mirroring is currently non-functional since we have made
> > the move to the ASF. ). There's also a certain class of folks that
> > have have sent patches via email that were also easy to apply.
> >
> > However, a number of folks are sitting behind servers or services that
> > actively break patches which has led to much gnashing of teeth here.
> > While I don't care so much about MUA issues, I do desperately despise
> > seeing MTAs breaking patches - and I spent around 4 hours last night
> > trying to unbreak patches from 3 different developers that their MTA
> > (all different) had made completely unusable.
> >
> > Where this really disturbs me is the barrier to participation. Working
> > on CloudStack is a pretty large barrier to begin with. You have to
> > understand the facet of CloudStack that you are working on, and then
> > understand git.
> >
> > While many of us can work via email - and some of us (me included)
> > even prefer it, I do worry about what the net effect is now that we
> > say - git patches submitted this way are actively broken by exchange,
> > appear to be broken by gmail, etc. I suppose we could push folks to
> > use some external mail service that behaves properly, but it seems
> > like an artificial barrier, and one that is largely out of the control
> > of folks wishing to collaborate with us.
> >
> > I think there are potentially benefits to using some tool other than
> > email down the road as well such as being able to have $test_suite run
> > against any patch before a committer even gets to review it.
> >
> > Thoughts, comments, flames?
> >
> > --David
> >
> Personally, I've always submitted patches via attaching them to bug
> reports.  Works well when I find a bug in something, don't have time to
> wait on anyone else to fix it, so I fix it myself, attach it to a bug
> report, and hope it's in the next release so I don't have to deal with
> it again.  Works pretty good with most open source projects.
> 
> Fred Wittekind

Re: Patches review

[Brett Porter <br...@apache.org>]

On 14/06/2012, at 1:09 AM, David Nalley wrote:

>>> So I have seen a lot of folks who use this approach, but that
>>> typically means that the mailing list gets cced on every action in the
>>> bugtracker. (mailing lists are where everything happens in Apache
>>> projects) We are already on track to hit 1,000 messages on this list
>>> alone this month - are we sure we want to add Jira traffic to that
>>> volume?
>>> 
>>> --David
>>> 
>> If we don't use the project's bug tracker to track the progress of bugs
>> and there patches, doesn't that defeat the purpose of having it?
>> 
>> Keeping the patch file attachments in Jira would keep those file
>> attachments out of the mailing list (reduction of traffic), and we
>> wouldn't run into MTA/MUAs mangling them.
>> 
>> If someone makes a comment in Jira, then CCs the mailing list, that
>> isn't any more mailing list traffic than sending to the same thing to
>> the mailing list alone.
>> 
>> Fred Wittekind
> 
> 
> Those are compelling arguments.

There's also no problem setting up additional lists if needed so that people on the dev list can focus on development discussion. Many projects have an "issues" dedicated list, or send their issue notifications to the commits list, to better segregate traffic. Either way, it is reasonably easy to filter messages of a certain type based on the sender and headers.

- Brett

--
Brett Porter
brett@apache.org
http://brettporter.wordpress.com/
http://au.linkedin.com/in/brettporter
http://twitter.com/brettporter

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Wed, Jun 13, 2012 at 10:49 AM, Fred Wittekind <ro...@twister.dyndns.org> wrote:
> On 6/13/2012 10:25 AM, David Nalley wrote:
>> On Wed, Jun 13, 2012 at 8:59 AM, Fred Wittekind <ro...@twister.dyndns.org> wrote:
>>> On 6/12/2012 4:01 PM, David Nalley wrote:
>>>> On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
>>>> <Al...@citrix.com> wrote:
>>>>> I know it's been discussed in several email threads, but I would like to
>>>>> initiate a separate discussion on what tool we should use for reviweing
>>>>> the patches.
>>>>>
>>>>> Several people (including myself - using Outlook on Mac OS X Lion) have
>>>>> been struggling already with applying email patches using "git am". Some
>>>>> patches appear to be broken, email file import/save is different in
>>>>> various email clients, etc. But the main disadvantage - there is no other
>>>>> way to track patch flow history rather than gathering email by subject.
>>>>> For instance, I would like to see the patch history in some centralized
>>>>> place:
>>>>>
>>>>> * when patch was created
>>>>> * who picked up the patch for the review and when
>>>>> * what was fixed after first, second,...n review
>>>>> * when the patch was merged to the mainstream.
>>>>>
>>>>> I think we should start using the official tool for that -
>>>>> Gerrit/Reviewboard/etc.
>>>>>
>>>>> Please follow up with your suggestions and preferences.
>>>>>
>>>>> -Alena.
>>>>>
>>>>>
>>>> Thanks for starting this - I have a thrice rewritten mail sitting in
>>>> my drafts folder around this subject. Quick followup to voice some of
>>>> my frustrations.
>>>>
>>>> We have a process today - and for a number of folks that has worked
>>>> very well. I personally find it dead easy to grab patches from github
>>>> (though our mirroring is currently non-functional since we have made
>>>> the move to the ASF. ). There's also a certain class of folks that
>>>> have have sent patches via email that were also easy to apply.
>>>>
>>>> However, a number of folks are sitting behind servers or services that
>>>> actively break patches which has led to much gnashing of teeth here.
>>>> While I don't care so much about MUA issues, I do desperately despise
>>>> seeing MTAs breaking patches - and I spent around 4 hours last night
>>>> trying to unbreak patches from 3 different developers that their MTA
>>>> (all different) had made completely unusable.
>>>>
>>>> Where this really disturbs me is the barrier to participation. Working
>>>> on CloudStack is a pretty large barrier to begin with. You have to
>>>> understand the facet of CloudStack that you are working on, and then
>>>> understand git.
>>>>
>>>> While many of us can work via email - and some of us (me included)
>>>> even prefer it, I do worry about what the net effect is now that we
>>>> say - git patches submitted this way are actively broken by exchange,
>>>> appear to be broken by gmail, etc. I suppose we could push folks to
>>>> use some external mail service that behaves properly, but it seems
>>>> like an artificial barrier, and one that is largely out of the control
>>>> of folks wishing to collaborate with us.
>>>>
>>>> I think there are potentially benefits to using some tool other than
>>>> email down the road as well such as being able to have $test_suite run
>>>> against any patch before a committer even gets to review it.
>>>>
>>>> Thoughts, comments, flames?
>>>>
>>>> --David
>>>>
>>> Personally, I've always submitted patches via attaching them to bug
>>> reports.  Works well when I find a bug in something, don't have time to
>>> wait on anyone else to fix it, so I fix it myself, attach it to a bug
>>> report, and hope it's in the next release so I don't have to deal with
>>> it again.  Works pretty good with most open source projects.
>>>
>>> Fred Wittekind
>>>
>> So I have seen a lot of folks who use this approach, but that
>> typically means that the mailing list gets cced on every action in the
>> bugtracker. (mailing lists are where everything happens in Apache
>> projects) We are already on track to hit 1,000 messages on this list
>> alone this month - are we sure we want to add Jira traffic to that
>> volume?
>>
>> --David
>>
> If we don't use the project's bug tracker to track the progress of bugs
> and there patches, doesn't that defeat the purpose of having it?
>
> Keeping the patch file attachments in Jira would keep those file
> attachments out of the mailing list (reduction of traffic), and we
> wouldn't run into MTA/MUAs mangling them.
>
> If someone makes a comment in Jira, then CCs the mailing list, that
> isn't any more mailing list traffic than sending to the same thing to
> the mailing list alone.
>
> Fred Wittekind


Those are compelling arguments.

--David

Re: Patches review

[John Kinsella <jl...@stratosec.co>]

On Jun 18, 2012, at 1:06 PM, Ewan Mellor wrote:

>> -----Original Message-----
>> From: Fred Wittekind [mailto:rom@twister.dyndns.org]
>> 
>> ...
>> 
>>> Personally, I've always submitted patches via attaching them to bug
>>>> reports.  Works well when I find a bug in something, don't have time
>> to
>>>> wait on anyone else to fix it, so I fix it myself, attach it to a
>> bug
>>>> report, and hope it's in the next release so I don't have to deal
>> with
>>>> it again.  Works pretty good with most open source projects.
>>>> 
>>>> Fred Wittekind
>>>> 
>>> So I have seen a lot of folks who use this approach, but that
>>> typically means that the mailing list gets cced on every action in
>> the
>>> bugtracker. (mailing lists are where everything happens in Apache
>>> projects) We are already on track to hit 1,000 messages on this list
>>> alone this month - are we sure we want to add Jira traffic to that
>>> volume?
>>> 
>>> --David
>>> 
>> If we don't use the project's bug tracker to track the progress of bugs
>> and there patches, doesn't that defeat the purpose of having it?
>> 
>> Keeping the patch file attachments in Jira would keep those file
>> attachments out of the mailing list (reduction of traffic), and we
>> wouldn't run into MTA/MUAs mangling them.
>> 
>> If someone makes a comment in Jira, then CCs the mailing list, that
>> isn't any more mailing list traffic than sending to the same thing to
>> the mailing list alone.
> 
> Hi,
> 
> I want to keep this thread alive, because this is an important decision in front of us, and the thread died on Wednesday without getting very far.
> 
> I think we're all agreed that we want to get patches out of email and into a tool that's better designed for peer review, automated test, and merge.  So that's the decision that's ahead of us -- what tool do we want to use for this?
> 
> In my opinion, Jira is a _fantastic_ bug tracker, but it's a poor tool for reviewing patches.  The best systems that I have seen will use a dedicated review tool, and will reflect details back to the bug tracker for archive.  That way, anyone looking at the bug can find the review discussion and see when a fix was merged, but the actual review itself can happen in a tool designed for the job.
> 
> I know of two decent options: Gerrit from the Google Android team, and ReviewBoard.  I've seen Gerrit used very successfully in the past.  I don't know anything about ReviewBoard, other than the fact that there is an instance hosted at reviews.apache.org.  (It was also down last week, which is a concern, but I'm sure we could address any instability problems if we wanted to depend on it.)
> 
> Does anyone have any other tools that we should look at, or comments on either Gerrit or ReviewBoard?
> 
> The next step from here would be to pick one or two to evaluate, and put together a workflow for patch acceptance that we can all agree on.

Atlassian's Crucible is decent, and would tie in with Jira pretty well…

Re: Patches review

[David Nalley <da...@gnsa.us>]

Hi folks,

It's been about a week since Review Board was stood up.

Anyone have any comments?

I have a couple - first compared to gerrit, ReviewBoard def lacks a
few features, though it's virtually zero effort for us to maintain. I
think it does provide a nice centralized place to look for patches,
but at the moment, that's about it. So it's better than email (where
50% of the patches submitted ended up being mangled).

I personally would like to see something pretty heavily automated to
at least verify:
* Patch applies cleanly
* Patched code builds

I'd also like for it to automate some testing at some point - though
the above two seem like lower hanging fruit.
So that bears the question do we want to try and stand up gerrit?
Anyone in love with ReviewBoard?

--David

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Wed, Jun 20, 2012 at 12:07 PM, Chip Childers
<ch...@sungard.com> wrote:
> On Wed, Jun 20, 2012 at 12:18 AM, David Nalley <da...@gnsa.us> wrote:
>> You can sign up for an account on reviews.a.o -
>> I haven't figured out workflow, so I might submit a patch or two just
>> to see how it works.
>
> I added 8 more (license header updates).  It would be great if we
> could use these to test the process and get the changes committed to
> the master ASF branch.
>
> -chip

Yeah, so quick status update - I've worked through one review via web
interface and meh....the diff is nice, but..

What I really wanted to use is post-review, but I haven't yet
succeeded in getting it to work - but that could be my own ignorance.
I've filed a ticket with infra about that as well.

--David

Re: Patches review

[Chip Childers <ch...@sungard.com>]

On Wed, Jun 20, 2012 at 12:18 AM, David Nalley <da...@gnsa.us> wrote:
> You can sign up for an account on reviews.a.o -
> I haven't figured out workflow, so I might submit a patch or two just
> to see how it works.

I added 8 more (license header updates).  It would be great if we
could use these to test the process and get the changes committed to
the master ASF branch.

-chip

Re: Patches review

[David Nalley <da...@gnsa.us>]

>
>
> Created:
> https://issues.apache.org/jira/browse/INFRA-4939
>
> --David

This is done (Thanks to the fine folks in INFRA!)

You can sign up for an account on reviews.a.o -
I haven't figured out workflow, so I might submit a patch or two just
to see how it works.

--David

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Tue, Jun 19, 2012 at 2:33 PM, Ewan Mellor <Ew...@eu.citrix.com> wrote:
>> -----Original Message-----
>> From: David Nalley [mailto:david@gnsa.us]
>>
>> On Jun 19, 2012, at 12:44 AM, Ewan Mellor <Ew...@eu.citrix.com>
>> wrote:
>>
>> >> [Re ReviewBoard]
>> >>
>> >> I think it's low 'cost' to try it out. If no one objects in the next
>> >> day or so I'll request that we get CloudStack instance in
>> reviewboard,
>> >> and we can try using it for a week or two and see if that is to our
>> >> liking. If there are specific problems that we find from a workflow
>> >> perspective that would be solved by Gerrit, we'll be in a better
>> place
>> >> to decide.
>> >>
>> >> (I see no problem with iteration here, as long as we recognize
>> failure
>> >> early on and then do something different)
>> >
>> > Sounds good to me.
>> >
>> > Can anyone comment on why it was down for so long recently?  We
>> obviously can't tolerate downtime like that if we're going to depend on
>> it.  I'd want to make sure that we address any stability problems
>> sooner rather than later.
>> >
>> > Thanks,
>> >
>> > Ewan.
>> >
>>
>> Yes, so briefly the downtime was caused by a failed upgrade which had
>> DB issues. Infra was essentially trying to avoid loss of historical
>> data. The upgrade was from 1.5.5 to 1.6.7, which upstream considered to
>> be a major upgrade, and there have been lots of reports of upgrade
>> problems based on a brief perusal of the mailing list. So not a
>> stability issue per se as much as a failed upgrade.
>
> OK, no big worries there then.  These things happen.
>
> I think that we're agreed.  We will try ReviewBoard first, because it's already maintained for us.  If it turns out that people don't like it enough to keep it, then our next phase would be to evaluate Gerrit and Crucible and see how those go.
>
> David, please go ahead and request our section on ReviewBoard.  If you know of any workflow docs that other projects are using, then would be helpful too.
>
> We'll start to use ReviewBoard for all patch acceptance as soon as it is ready and the committers have agreed a workflow.
>
> Thanks,
>
> Ewan.
>



Created:
https://issues.apache.org/jira/browse/INFRA-4939

--David

RE: Patches review

[Ewan Mellor <Ew...@eu.citrix.com>]

> -----Original Message-----
> From: David Nalley [mailto:david@gnsa.us]
>
> On Jun 19, 2012, at 12:44 AM, Ewan Mellor <Ew...@eu.citrix.com>
> wrote:
> 
> >> [Re ReviewBoard]
> >>
> >> I think it's low 'cost' to try it out. If no one objects in the next
> >> day or so I'll request that we get CloudStack instance in
> reviewboard,
> >> and we can try using it for a week or two and see if that is to our
> >> liking. If there are specific problems that we find from a workflow
> >> perspective that would be solved by Gerrit, we'll be in a better
> place
> >> to decide.
> >>
> >> (I see no problem with iteration here, as long as we recognize
> failure
> >> early on and then do something different)
> >
> > Sounds good to me.
> >
> > Can anyone comment on why it was down for so long recently?  We
> obviously can't tolerate downtime like that if we're going to depend on
> it.  I'd want to make sure that we address any stability problems
> sooner rather than later.
> >
> > Thanks,
> >
> > Ewan.
> >
> 
> Yes, so briefly the downtime was caused by a failed upgrade which had
> DB issues. Infra was essentially trying to avoid loss of historical
> data. The upgrade was from 1.5.5 to 1.6.7, which upstream considered to
> be a major upgrade, and there have been lots of reports of upgrade
> problems based on a brief perusal of the mailing list. So not a
> stability issue per se as much as a failed upgrade.

OK, no big worries there then.  These things happen.

I think that we're agreed.  We will try ReviewBoard first, because it's already maintained for us.  If it turns out that people don't like it enough to keep it, then our next phase would be to evaluate Gerrit and Crucible and see how those go.

David, please go ahead and request our section on ReviewBoard.  If you know of any workflow docs that other projects are using, then would be helpful too.

We'll start to use ReviewBoard for all patch acceptance as soon as it is ready and the committers have agreed a workflow.

Thanks,

Ewan.

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Jun 19, 2012, at 12:44 AM, Ewan Mellor <Ew...@eu.citrix.com> wrote:

>> [Re ReviewBoard]
>> 
>> I think it's low 'cost' to try it out. If no one objects in the next
>> day or so I'll request that we get CloudStack instance in reviewboard,
>> and we can try using it for a week or two and see if that is to our
>> liking. If there are specific problems that we find from a workflow
>> perspective that would be solved by Gerrit, we'll be in a better place
>> to decide.
>> 
>> (I see no problem with iteration here, as long as we recognize failure
>> early on and then do something different)
> 
> Sounds good to me.
> 
> Can anyone comment on why it was down for so long recently?  We obviously can't tolerate downtime like that if we're going to depend on it.  I'd want to make sure that we address any stability problems sooner rather than later.
> 
> Thanks,
> 
> Ewan.
> 

Yes, so briefly the downtime was caused by a failed upgrade which had DB issues. Infra was essentially trying to avoid loss of historical data. The upgrade was from 1.5.5 to 1.6.7, which upstream considered to be a major upgrade, and there have been lots of reports of upgrade problems based on a brief perusal of the mailing list. So not a stability issue per se as much as a failed upgrade. 

--David

RE: Patches review

[Ewan Mellor <Ew...@eu.citrix.com>]

> [Re ReviewBoard]
>
> I think it's low 'cost' to try it out. If no one objects in the next
> day or so I'll request that we get CloudStack instance in reviewboard,
> and we can try using it for a week or two and see if that is to our
> liking. If there are specific problems that we find from a workflow
> perspective that would be solved by Gerrit, we'll be in a better place
> to decide.
> 
> (I see no problem with iteration here, as long as we recognize failure
> early on and then do something different)

Sounds good to me.

Can anyone comment on why it was down for so long recently?  We obviously can't tolerate downtime like that if we're going to depend on it.  I'd want to make sure that we address any stability problems sooner rather than later.

Thanks,

Ewan.

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Mon, Jun 18, 2012 at 8:04 PM, Kevin Kluge <Ke...@citrix.com> wrote:
>> -----Original Message-----
>> From: David Nalley [mailto:david@gnsa.us]
>> Sent: Monday, June 18, 2012 2:20 PM
>> To: cloudstack-dev@incubator.apache.org
>> Subject: Re: Patches review
>>
>> On Mon, Jun 18, 2012 at 4:17 PM, Adrian Cole <ad...@gmail.com>
>> wrote:
>> > +1 gerrit, particularly as it helps avoid additional process
>> > +mismatches for
>> > folks working in both openstack and cloudstack
>>
>> So while I like the thinking here, let me present the flip side of the coin.
>> Reviewboard is what is used in a large number of other ASF projects, so
>> getting reviewboard up and running is just a ticket. Whereas gerrit would
>> mean we would need to have a set of committers maintain that
>> infrastructure. Not necessarily a blocker, but something else to consider. This
>> also means we could likely get reviewboard up in short order, while gerrit
>> could potentially take longer as we have to both do the work, and wait for
>> allocation of resources.
>
> If that's the case can we default to ReviewBoard, and choose something else only if there is strong objection?
>
> Does anyone have anything +/- to say from actual use of ReviewBoard?
>
> -kevin
>
>

I think it's low 'cost' to try it out. If no one objects in the next
day or so I'll request that we get CloudStack instance in reviewboard,
and we can try using it for a week or two and see if that is to our
liking. If there are specific problems that we find from a workflow
perspective that would be solved by Gerrit, we'll be in a better place
to decide.

(I see no problem with iteration here, as long as we recognize failure
early on and then do something different)

--David

RE: Patches review

[Kevin Kluge <Ke...@citrix.com>]

> -----Original Message-----
> From: David Nalley [mailto:david@gnsa.us]
> Sent: Monday, June 18, 2012 2:20 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Patches review
> 
> On Mon, Jun 18, 2012 at 4:17 PM, Adrian Cole <ad...@gmail.com>
> wrote:
> > +1 gerrit, particularly as it helps avoid additional process
> > +mismatches for
> > folks working in both openstack and cloudstack
> 
> So while I like the thinking here, let me present the flip side of the coin.
> Reviewboard is what is used in a large number of other ASF projects, so
> getting reviewboard up and running is just a ticket. Whereas gerrit would
> mean we would need to have a set of committers maintain that
> infrastructure. Not necessarily a blocker, but something else to consider. This
> also means we could likely get reviewboard up in short order, while gerrit
> could potentially take longer as we have to both do the work, and wait for
> allocation of resources.

If that's the case can we default to ReviewBoard, and choose something else only if there is strong objection?    

Does anyone have anything +/- to say from actual use of ReviewBoard?

-kevin

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Mon, Jun 18, 2012 at 4:17 PM, Adrian Cole <ad...@gmail.com> wrote:
> +1 gerrit, particularly as it helps avoid additional process mismatches for
> folks working in both openstack and cloudstack

So while I like the thinking here, let me present the flip side of the coin.
Reviewboard is what is used in a large number of other ASF projects,
so getting reviewboard up and running is just a ticket. Whereas gerrit
would mean we would need to have a set of committers maintain that
infrastructure. Not necessarily a blocker, but something else to
consider. This also means we could likely get reviewboard up in short
order, while gerrit could potentially take longer as we have to both
do the work, and wait for allocation of resources.

--David

RE: Patches review

[Adrian Cole <ad...@gmail.com>]

+1 gerrit, particularly as it helps avoid additional process mismatches for
folks working in both openstack and cloudstack
On Jun 18, 2012 2:07 PM, "Ewan Mellor" <Ew...@eu.citrix.com> wrote:

> > -----Original Message-----
> > From: Fred Wittekind [mailto:rom@twister.dyndns.org]
> >
> > ...
> >
> >> Personally, I've always submitted patches via attaching them to bug
> > >> reports.  Works well when I find a bug in something, don't have time
> > to
> > >> wait on anyone else to fix it, so I fix it myself, attach it to a
> > bug
> > >> report, and hope it's in the next release so I don't have to deal
> > with
> > >> it again.  Works pretty good with most open source projects.
> > >>
> > >> Fred Wittekind
> > >>
> > > So I have seen a lot of folks who use this approach, but that
> > > typically means that the mailing list gets cced on every action in
> > the
> > > bugtracker. (mailing lists are where everything happens in Apache
> > > projects) We are already on track to hit 1,000 messages on this list
> > > alone this month - are we sure we want to add Jira traffic to that
> > > volume?
> > >
> > > --David
> > >
> > If we don't use the project's bug tracker to track the progress of bugs
> > and there patches, doesn't that defeat the purpose of having it?
> >
> > Keeping the patch file attachments in Jira would keep those file
> > attachments out of the mailing list (reduction of traffic), and we
> > wouldn't run into MTA/MUAs mangling them.
> >
> > If someone makes a comment in Jira, then CCs the mailing list, that
> > isn't any more mailing list traffic than sending to the same thing to
> > the mailing list alone.
>
> Hi,
>
> I want to keep this thread alive, because this is an important decision in
> front of us, and the thread died on Wednesday without getting very far.
>
> I think we're all agreed that we want to get patches out of email and into
> a tool that's better designed for peer review, automated test, and merge.
>  So that's the decision that's ahead of us -- what tool do we want to use
> for this?
>
> In my opinion, Jira is a _fantastic_ bug tracker, but it's a poor tool for
> reviewing patches.  The best systems that I have seen will use a dedicated
> review tool, and will reflect details back to the bug tracker for archive.
>  That way, anyone looking at the bug can find the review discussion and see
> when a fix was merged, but the actual review itself can happen in a tool
> designed for the job.
>
> I know of two decent options: Gerrit from the Google Android team, and
> ReviewBoard.  I've seen Gerrit used very successfully in the past.  I don't
> know anything about ReviewBoard, other than the fact that there is an
> instance hosted at reviews.apache.org.  (It was also down last week,
> which is a concern, but I'm sure we could address any instability problems
> if we wanted to depend on it.)
>
> Does anyone have any other tools that we should look at, or comments on
> either Gerrit or ReviewBoard?
>
> The next step from here would be to pick one or two to evaluate, and put
> together a workflow for patch acceptance that we can all agree on.
>
> Thanks,
>
> Ewan.
>
>

RE: Patches review

[Ewan Mellor <Ew...@eu.citrix.com>]

> -----Original Message-----
> From: Fred Wittekind [mailto:rom@twister.dyndns.org]
>
> ...
>
>> Personally, I've always submitted patches via attaching them to bug
> >> reports.  Works well when I find a bug in something, don't have time
> to
> >> wait on anyone else to fix it, so I fix it myself, attach it to a
> bug
> >> report, and hope it's in the next release so I don't have to deal
> with
> >> it again.  Works pretty good with most open source projects.
> >>
> >> Fred Wittekind
> >>
> > So I have seen a lot of folks who use this approach, but that
> > typically means that the mailing list gets cced on every action in
> the
> > bugtracker. (mailing lists are where everything happens in Apache
> > projects) We are already on track to hit 1,000 messages on this list
> > alone this month - are we sure we want to add Jira traffic to that
> > volume?
> >
> > --David
> >
> If we don't use the project's bug tracker to track the progress of bugs
> and there patches, doesn't that defeat the purpose of having it?
> 
> Keeping the patch file attachments in Jira would keep those file
> attachments out of the mailing list (reduction of traffic), and we
> wouldn't run into MTA/MUAs mangling them.
> 
> If someone makes a comment in Jira, then CCs the mailing list, that
> isn't any more mailing list traffic than sending to the same thing to
> the mailing list alone.

Hi,

I want to keep this thread alive, because this is an important decision in front of us, and the thread died on Wednesday without getting very far.

I think we're all agreed that we want to get patches out of email and into a tool that's better designed for peer review, automated test, and merge.  So that's the decision that's ahead of us -- what tool do we want to use for this?

In my opinion, Jira is a _fantastic_ bug tracker, but it's a poor tool for reviewing patches.  The best systems that I have seen will use a dedicated review tool, and will reflect details back to the bug tracker for archive.  That way, anyone looking at the bug can find the review discussion and see when a fix was merged, but the actual review itself can happen in a tool designed for the job.

I know of two decent options: Gerrit from the Google Android team, and ReviewBoard.  I've seen Gerrit used very successfully in the past.  I don't know anything about ReviewBoard, other than the fact that there is an instance hosted at reviews.apache.org.  (It was also down last week, which is a concern, but I'm sure we could address any instability problems if we wanted to depend on it.)

Does anyone have any other tools that we should look at, or comments on either Gerrit or ReviewBoard?

The next step from here would be to pick one or two to evaluate, and put together a workflow for patch acceptance that we can all agree on.

Thanks,

Ewan.

Re: Patches review

[Fred Wittekind <ro...@twister.dyndns.org>]

On 6/13/2012 10:25 AM, David Nalley wrote:
> On Wed, Jun 13, 2012 at 8:59 AM, Fred Wittekind <ro...@twister.dyndns.org> wrote:
>> On 6/12/2012 4:01 PM, David Nalley wrote:
>>> On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
>>> <Al...@citrix.com> wrote:
>>>> I know it's been discussed in several email threads, but I would like to
>>>> initiate a separate discussion on what tool we should use for reviweing
>>>> the patches.
>>>>
>>>> Several people (including myself - using Outlook on Mac OS X Lion) have
>>>> been struggling already with applying email patches using "git am". Some
>>>> patches appear to be broken, email file import/save is different in
>>>> various email clients, etc. But the main disadvantage - there is no other
>>>> way to track patch flow history rather than gathering email by subject.
>>>> For instance, I would like to see the patch history in some centralized
>>>> place:
>>>>
>>>> * when patch was created
>>>> * who picked up the patch for the review and when
>>>> * what was fixed after first, second,...n review
>>>> * when the patch was merged to the mainstream.
>>>>
>>>> I think we should start using the official tool for that -
>>>> Gerrit/Reviewboard/etc.
>>>>
>>>> Please follow up with your suggestions and preferences.
>>>>
>>>> -Alena.
>>>>
>>>>
>>> Thanks for starting this - I have a thrice rewritten mail sitting in
>>> my drafts folder around this subject. Quick followup to voice some of
>>> my frustrations.
>>>
>>> We have a process today - and for a number of folks that has worked
>>> very well. I personally find it dead easy to grab patches from github
>>> (though our mirroring is currently non-functional since we have made
>>> the move to the ASF. ). There's also a certain class of folks that
>>> have have sent patches via email that were also easy to apply.
>>>
>>> However, a number of folks are sitting behind servers or services that
>>> actively break patches which has led to much gnashing of teeth here.
>>> While I don't care so much about MUA issues, I do desperately despise
>>> seeing MTAs breaking patches - and I spent around 4 hours last night
>>> trying to unbreak patches from 3 different developers that their MTA
>>> (all different) had made completely unusable.
>>>
>>> Where this really disturbs me is the barrier to participation. Working
>>> on CloudStack is a pretty large barrier to begin with. You have to
>>> understand the facet of CloudStack that you are working on, and then
>>> understand git.
>>>
>>> While many of us can work via email - and some of us (me included)
>>> even prefer it, I do worry about what the net effect is now that we
>>> say - git patches submitted this way are actively broken by exchange,
>>> appear to be broken by gmail, etc. I suppose we could push folks to
>>> use some external mail service that behaves properly, but it seems
>>> like an artificial barrier, and one that is largely out of the control
>>> of folks wishing to collaborate with us.
>>>
>>> I think there are potentially benefits to using some tool other than
>>> email down the road as well such as being able to have $test_suite run
>>> against any patch before a committer even gets to review it.
>>>
>>> Thoughts, comments, flames?
>>>
>>> --David
>>>
>> Personally, I've always submitted patches via attaching them to bug
>> reports.  Works well when I find a bug in something, don't have time to
>> wait on anyone else to fix it, so I fix it myself, attach it to a bug
>> report, and hope it's in the next release so I don't have to deal with
>> it again.  Works pretty good with most open source projects.
>>
>> Fred Wittekind
>>
> So I have seen a lot of folks who use this approach, but that
> typically means that the mailing list gets cced on every action in the
> bugtracker. (mailing lists are where everything happens in Apache
> projects) We are already on track to hit 1,000 messages on this list
> alone this month - are we sure we want to add Jira traffic to that
> volume?
>
> --David
>
If we don't use the project's bug tracker to track the progress of bugs
and there patches, doesn't that defeat the purpose of having it?

Keeping the patch file attachments in Jira would keep those file
attachments out of the mailing list (reduction of traffic), and we
wouldn't run into MTA/MUAs mangling them.

If someone makes a comment in Jira, then CCs the mailing list, that
isn't any more mailing list traffic than sending to the same thing to
the mailing list alone.

Fred Wittekind

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Wed, Jun 13, 2012 at 8:59 AM, Fred Wittekind <ro...@twister.dyndns.org> wrote:
> On 6/12/2012 4:01 PM, David Nalley wrote:
>> On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
>> <Al...@citrix.com> wrote:
>>> I know it's been discussed in several email threads, but I would like to
>>> initiate a separate discussion on what tool we should use for reviweing
>>> the patches.
>>>
>>> Several people (including myself - using Outlook on Mac OS X Lion) have
>>> been struggling already with applying email patches using "git am". Some
>>> patches appear to be broken, email file import/save is different in
>>> various email clients, etc. But the main disadvantage - there is no other
>>> way to track patch flow history rather than gathering email by subject.
>>> For instance, I would like to see the patch history in some centralized
>>> place:
>>>
>>> * when patch was created
>>> * who picked up the patch for the review and when
>>> * what was fixed after first, second,...n review
>>> * when the patch was merged to the mainstream.
>>>
>>> I think we should start using the official tool for that -
>>> Gerrit/Reviewboard/etc.
>>>
>>> Please follow up with your suggestions and preferences.
>>>
>>> -Alena.
>>>
>>>
>> Thanks for starting this - I have a thrice rewritten mail sitting in
>> my drafts folder around this subject. Quick followup to voice some of
>> my frustrations.
>>
>> We have a process today - and for a number of folks that has worked
>> very well. I personally find it dead easy to grab patches from github
>> (though our mirroring is currently non-functional since we have made
>> the move to the ASF. ). There's also a certain class of folks that
>> have have sent patches via email that were also easy to apply.
>>
>> However, a number of folks are sitting behind servers or services that
>> actively break patches which has led to much gnashing of teeth here.
>> While I don't care so much about MUA issues, I do desperately despise
>> seeing MTAs breaking patches - and I spent around 4 hours last night
>> trying to unbreak patches from 3 different developers that their MTA
>> (all different) had made completely unusable.
>>
>> Where this really disturbs me is the barrier to participation. Working
>> on CloudStack is a pretty large barrier to begin with. You have to
>> understand the facet of CloudStack that you are working on, and then
>> understand git.
>>
>> While many of us can work via email - and some of us (me included)
>> even prefer it, I do worry about what the net effect is now that we
>> say - git patches submitted this way are actively broken by exchange,
>> appear to be broken by gmail, etc. I suppose we could push folks to
>> use some external mail service that behaves properly, but it seems
>> like an artificial barrier, and one that is largely out of the control
>> of folks wishing to collaborate with us.
>>
>> I think there are potentially benefits to using some tool other than
>> email down the road as well such as being able to have $test_suite run
>> against any patch before a committer even gets to review it.
>>
>> Thoughts, comments, flames?
>>
>> --David
>>
> Personally, I've always submitted patches via attaching them to bug
> reports.  Works well when I find a bug in something, don't have time to
> wait on anyone else to fix it, so I fix it myself, attach it to a bug
> report, and hope it's in the next release so I don't have to deal with
> it again.  Works pretty good with most open source projects.
>
> Fred Wittekind
>

So I have seen a lot of folks who use this approach, but that
typically means that the mailing list gets cced on every action in the
bugtracker. (mailing lists are where everything happens in Apache
projects) We are already on track to hit 1,000 messages on this list
alone this month - are we sure we want to add Jira traffic to that
volume?

--David

Re: Patches review

[Fred Wittekind <ro...@twister.dyndns.org>]

On 6/12/2012 4:01 PM, David Nalley wrote:
> On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
> <Al...@citrix.com> wrote:
>> I know it's been discussed in several email threads, but I would like to
>> initiate a separate discussion on what tool we should use for reviweing
>> the patches.
>>
>> Several people (including myself - using Outlook on Mac OS X Lion) have
>> been struggling already with applying email patches using "git am". Some
>> patches appear to be broken, email file import/save is different in
>> various email clients, etc. But the main disadvantage - there is no other
>> way to track patch flow history rather than gathering email by subject.
>> For instance, I would like to see the patch history in some centralized
>> place:
>>
>> * when patch was created
>> * who picked up the patch for the review and when
>> * what was fixed after first, second,...n review
>> * when the patch was merged to the mainstream.
>>
>> I think we should start using the official tool for that -
>> Gerrit/Reviewboard/etc.
>>
>> Please follow up with your suggestions and preferences.
>>
>> -Alena.
>>
>>
> Thanks for starting this - I have a thrice rewritten mail sitting in
> my drafts folder around this subject. Quick followup to voice some of
> my frustrations.
>
> We have a process today - and for a number of folks that has worked
> very well. I personally find it dead easy to grab patches from github
> (though our mirroring is currently non-functional since we have made
> the move to the ASF. ). There's also a certain class of folks that
> have have sent patches via email that were also easy to apply.
>
> However, a number of folks are sitting behind servers or services that
> actively break patches which has led to much gnashing of teeth here.
> While I don't care so much about MUA issues, I do desperately despise
> seeing MTAs breaking patches - and I spent around 4 hours last night
> trying to unbreak patches from 3 different developers that their MTA
> (all different) had made completely unusable.
>
> Where this really disturbs me is the barrier to participation. Working
> on CloudStack is a pretty large barrier to begin with. You have to
> understand the facet of CloudStack that you are working on, and then
> understand git.
>
> While many of us can work via email - and some of us (me included)
> even prefer it, I do worry about what the net effect is now that we
> say - git patches submitted this way are actively broken by exchange,
> appear to be broken by gmail, etc. I suppose we could push folks to
> use some external mail service that behaves properly, but it seems
> like an artificial barrier, and one that is largely out of the control
> of folks wishing to collaborate with us.
>
> I think there are potentially benefits to using some tool other than
> email down the road as well such as being able to have $test_suite run
> against any patch before a committer even gets to review it.
>
> Thoughts, comments, flames?
>
> --David
>
Personally, I've always submitted patches via attaching them to bug
reports.  Works well when I find a bug in something, don't have time to
wait on anyone else to fix it, so I fix it myself, attach it to a bug
report, and hope it's in the next release so I don't have to deal with
it again.  Works pretty good with most open source projects.

Fred Wittekind

Re: Patches review

[David Nalley <da...@gnsa.us>]

On Tue, Jun 12, 2012 at 3:46 PM, Alena Prokharchyk
<Al...@citrix.com> wrote:
> I know it's been discussed in several email threads, but I would like to
> initiate a separate discussion on what tool we should use for reviweing
> the patches.
>
> Several people (including myself - using Outlook on Mac OS X Lion) have
> been struggling already with applying email patches using "git am". Some
> patches appear to be broken, email file import/save is different in
> various email clients, etc. But the main disadvantage - there is no other
> way to track patch flow history rather than gathering email by subject.
> For instance, I would like to see the patch history in some centralized
> place:
>
> * when patch was created
> * who picked up the patch for the review and when
> * what was fixed after first, second,...n review
> * when the patch was merged to the mainstream.
>
> I think we should start using the official tool for that -
> Gerrit/Reviewboard/etc.
>
> Please follow up with your suggestions and preferences.
>
> -Alena.
>
>

Thanks for starting this - I have a thrice rewritten mail sitting in
my drafts folder around this subject. Quick followup to voice some of
my frustrations.

We have a process today - and for a number of folks that has worked
very well. I personally find it dead easy to grab patches from github
(though our mirroring is currently non-functional since we have made
the move to the ASF. ). There's also a certain class of folks that
have have sent patches via email that were also easy to apply.

However, a number of folks are sitting behind servers or services that
actively break patches which has led to much gnashing of teeth here.
While I don't care so much about MUA issues, I do desperately despise
seeing MTAs breaking patches - and I spent around 4 hours last night
trying to unbreak patches from 3 different developers that their MTA
(all different) had made completely unusable.

Where this really disturbs me is the barrier to participation. Working
on CloudStack is a pretty large barrier to begin with. You have to
understand the facet of CloudStack that you are working on, and then
understand git.

While many of us can work via email - and some of us (me included)
even prefer it, I do worry about what the net effect is now that we
say - git patches submitted this way are actively broken by exchange,
appear to be broken by gmail, etc. I suppose we could push folks to
use some external mail service that behaves properly, but it seems
like an artificial barrier, and one that is largely out of the control
of folks wishing to collaborate with us.

I think there are potentially benefits to using some tool other than
email down the road as well such as being able to have $test_suite run
against any patch before a committer even gets to review it.

Thoughts, comments, flames?

--David