You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Chip Childers <ch...@sungard.com> on 2013/02/20 23:03:37 UTC

Branch Merge Expectations - Draft for Discussion

Hi all,

I spent some time working on a draft of "branch merge expectations" [1],
and I'd really like to get feedback on it.

Did I document our consensus on the topic correctly?  Is everyone
comfortable with these guidelines?  Did I miss anything?

-chip

[1]
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Expectations

Re: Branch Merge Expectations - Draft for Discussion

Posted by Chip Childers <ch...@sungard.com>.
On Thu, Feb 21, 2013 at 07:36:21PM +0530, Prasanna Santhanam wrote:
> On Thu, Feb 21, 2013 at 03:33:37AM +0530, Chip Childers wrote:
> > Hi all,
> > 
> > I spent some time working on a draft of "branch merge expectations" [1],
> > and I'd really like to get feedback on it.
> > 
> > Did I document our consensus on the topic correctly?  Is everyone
> > comfortable with these guidelines?  Did I miss anything?
> > 
> > -chip
> > 
> > [1]
> > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Expectations
> 
> May be a word about regularly merging master in to your feature branch
> during its development. Branches shouldn't become silos. Since others
> are developing features in their branches that could possibly affect
> your feature it's better to do this on a regular basis.

Added - Thx.

> 
> -- 
> Prasanna.,
> 

Re: Branch Merge Expectations - Draft for Discussion

Posted by Prasanna Santhanam <ts...@apache.org>.
On Thu, Feb 21, 2013 at 03:33:37AM +0530, Chip Childers wrote:
> Hi all,
> 
> I spent some time working on a draft of "branch merge expectations" [1],
> and I'd really like to get feedback on it.
> 
> Did I document our consensus on the topic correctly?  Is everyone
> comfortable with these guidelines?  Did I miss anything?
> 
> -chip
> 
> [1]
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Expectations

May be a word about regularly merging master in to your feature branch
during its development. Branches shouldn't become silos. Since others
are developing features in their branches that could possibly affect
your feature it's better to do this on a regular basis.

-- 
Prasanna.,

Re: Branch Merge Expectations - Draft for Discussion

Posted by Chip Childers <ch...@sungard.com>.
On Wed, Feb 20, 2013 at 02:23:11PM -0800, Animesh Chaturvedi wrote:
> Chip
> 
> I posted this in another thread where 72 hour was called out,
> 

Yup - and I figured we could bring that discussion into this new thread.

> 
> Do we really need to wait 72 hours for all merge requests? I feel that slows developers down unless they plan very well. We had similar discussion just 3 weeks back and it was discussed that it depends on the scope and dependencies with other modules on case-by-case basis. If all discussion and development and review has happened by community and if it is isolated feature merge can be done quickly.
> 
> Here is link to that discussion.
> 
> http://mail-archives.apache.org/mod_mbox/incubator-cloudstack-dev/201301.mbox/%3C93099572B72EB341B81A644E134F240B012F747FE622@SJCPMAILBOX01.citrite.net%3E
> 

So here's what I said last time:

> Purely IMO, I think that if the discussion has already happened on the
> dev list
> about requirements and FS, the code has been looked at by others (or
> people have had the opportunity to look at it), you have tested and
> feel
> confident, then the MERGE request is really a notification to make
> sure folks
> know you are going to do it.  It will entirely depend on the nature
> (scope) of
> the changes.  An isolated feature can probably be merged very quickly
> with
> little debate.

The question is really "has the community had an opportunity to review
the code".  When I wrote that, I was frankly thinking along the lines
that committers would follow the "do the right thing" model.  However,
if we believe that we need to document expectations in a more formal way (as
requested by several others), then I think we need to document clear
guidelines that are easy to follow and are as unambiguous as possible.

So, again, IMO I'd be willing to be less structured about this if the
intent of keeping master "stable" and giving the community enough time
to review proposed changes is met.  But how do we codify that?  Do we
just include the intentions and goals in the wiki page, and leave it at
that?

I'm more than open to suggested edits!  Feel free suggest or make
updates to the draft.


> 
> 
> > -----Original Message-----
> > From: Chip Childers [mailto:chip.childers@sungard.com]
> > Sent: Wednesday, February 20, 2013 2:04 PM
> > To: cloudstack-dev@incubator.apache.org
> > Subject: Branch Merge Expectations - Draft for Discussion
> > 
> > Hi all,
> > 
> > I spent some time working on a draft of "branch merge expectations" [1], and
> > I'd really like to get feedback on it.
> > 
> > Did I document our consensus on the topic correctly?  Is everyone comfortable
> > with these guidelines?  Did I miss anything?
> > 
> > -chip
> > 
> > [1]
> > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Exp
> > ectations
> 

Re: Branch Merge Expectations - Draft for Discussion

Posted by David Nalley <da...@gnsa.us>.
On Wed, Feb 20, 2013 at 5:23 PM, Animesh Chaturvedi
<an...@citrix.com> wrote:
> Chip
>
> I posted this in another thread where 72 hour was called out,
>
>
> Do we really need to wait 72 hours for all merge requests? I feel that slows developers down unless they plan very well. We had similar discussion just 3 weeks back and it was discussed that it depends on the scope and dependencies with other modules on case-by-case basis. If all discussion and development and review has happened by community and if it is isolated feature merge can be done quickly.
>
> Here is link to that discussion.
>
> http://mail-archives.apache.org/mod_mbox/incubator-cloudstack-dev/201301.mbox/%3C93099572B72EB341B81A644E134F240B012F747FE622@SJCPMAILBOX01.citrite.net%3E
>
>


This is the problem with needing guidelines for things. It's much
easier to operate more informally, but the challenge, particularly
with a rapidly evolving and growing community is to communicate the
community culture and expectations effectively. One way to do that is
to have guidelines for everything, and it's probably the least
manpower intensive. The opposite end of that spectrum is to heavily
mentor folks.

One of our challenges is that we are very distributed, and the mailing
list is really our only common point of interaction, and with the
volume, it can be an ineffective learning process. We do have IRC, but
I'd guess that less than 25% of our committers hang out there - even
though we have around 150 people combined in #cloudstack and
#cloudstack-dev at any given time.

We do want to keep things as streamlined as possible, while not
sacrificing quality or community participation. I also get the sense
that rushing merges will result in far more vetos from people who
would have participated in a conversation about the merge; that's more
expensive in a time perspective than waiting a bit longer.

--David

Re: Branch Merge Expectations - Draft for Discussion

Posted by Marcus Sorensen <sh...@gmail.com>.
On Fri, Feb 22, 2013 at 8:42 AM, Chip Childers
<ch...@sungard.com> wrote:
> On Thu, Feb 21, 2013 at 11:55:27AM -0800, Animesh Chaturvedi wrote:
>>
>>
>> > -----Original Message-----
>> > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
>> > Sent: Thursday, February 21, 2013 9:02 AM
>> > To: cloudstack-dev@incubator.apache.org
>> > Subject: Re: Branch Merge Expectations - Draft for Discussion
>> >
>> > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
>> > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
>> > >> Do we really need to wait 72 hours for all merge requests? I feel
>> > >> that slows developers down unless they plan very well.
>> > >
>> > > What's wrong with the expectation being that they plan very well? ;-)
>> > >
>> > > Remember, "community over code." The point of waiting 72 hours is to
>> > > give the community the opportunity to review, comment, etc.
>> > >
>> > > The point that some merges are less disruptive / intrusive than others
>> > > is well-taken, though. Perhaps that is something that could be
>> > > discussed during the feature proposal and decided then. If the
>> > > community decides up-front that a merge is unlikely to be a problem,
>> > > then maybe the expectation would be that only 48 or 24 hours needs to
>> > > pass to allow for review & comments. But it should be explicit, and
>> > > I'd rather err on the side of allowing the community time to review.
>> >
>> > I think the idea is that the people that a review would be targeted at are likely
>> > already involved, or perhaps review has been requested independently prior to
>> > formally requesting the merge. So the question is whether it's necessary to
>> > open up a 72 hour window where the general dev team has a chance to review
>> > the code, when presumably all of the people who care should be involved, if the
>> > feature is progressing properly. I'm not entirely sure.
>> >
>> [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours. Those who need to be involved should be engaged early on throughout the development. If we push MERGE request as the formal mechanism for the community to review and respond it may be too late and I doubt how much of that will happen even in 72 hours.
>
> I agree with the comments that it shouldn't be the only time for review
> and discussion.
>
> The more I observe people interacting with master, the more I'd respond
> to this by asking:  Why are we ever in a rush to merge changes in?
>
> Shouldn't community consensus and master stability be more important
> than anything specific feature?

Probably, yes. But I can sympathize in some scenarios, as the rate of
development on cloudstack is pretty aggressive, and depending on what
you're working on, it can be a full time job just merging master into
your branch every so often and fixing/re-testing your branch.
Fundamental shifts like Javelin and the storage refactor, at some
point need the community to start working around them rather than
vice-versa, or you're stuck. Or alternatively, maybe it just means
that we are short on developers for that branch. Then there are
features branched off of those branches, that also have to deal with
the master pull cycle.

While we are on the subject of rushing merges, in retrospect, I do
think it was probably not a good idea to merge Javelin into 4.1, as
nothing in that branch really benefits from it, and everything that
does (or will) benefit goes on master. There have been a lot of issues
around it, which were probably expected, and 4.1 was not the best
place to guinea pig it.

So in general, yes, we only want to merge things that are complete,
with tests, etc. It should take as long as it takes. But I'd also like
to see us consider some of the more fundamental shifts during the
timeframe we're in now, when master has been recently branched for
release. Not to add to the bureaucracy, but perhaps we can have
different classes of feature branches, for example, large sweeping
changes like Javelin are only considered in the first 30 days of a dev
cycle, knowing that surely issues will arise even with good testing
efforts, while smaller individual features that are easier to test can
be merged throughout the whole cycle, but need to be close to perfect
when they do. I'm not sure how that classification would occur, just
thinking out loud.

>
> -chip

Re: Branch Merge Expectations - Draft for Discussion

Posted by Chip Childers <ch...@sungard.com>.
On Thu, Feb 21, 2013 at 11:55:27AM -0800, Animesh Chaturvedi wrote:
> 
> 
> > -----Original Message-----
> > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> > Sent: Thursday, February 21, 2013 9:02 AM
> > To: cloudstack-dev@incubator.apache.org
> > Subject: Re: Branch Merge Expectations - Draft for Discussion
> > 
> > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
> > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> > >> Do we really need to wait 72 hours for all merge requests? I feel
> > >> that slows developers down unless they plan very well.
> > >
> > > What's wrong with the expectation being that they plan very well? ;-)
> > >
> > > Remember, "community over code." The point of waiting 72 hours is to
> > > give the community the opportunity to review, comment, etc.
> > >
> > > The point that some merges are less disruptive / intrusive than others
> > > is well-taken, though. Perhaps that is something that could be
> > > discussed during the feature proposal and decided then. If the
> > > community decides up-front that a merge is unlikely to be a problem,
> > > then maybe the expectation would be that only 48 or 24 hours needs to
> > > pass to allow for review & comments. But it should be explicit, and
> > > I'd rather err on the side of allowing the community time to review.
> > 
> > I think the idea is that the people that a review would be targeted at are likely
> > already involved, or perhaps review has been requested independently prior to
> > formally requesting the merge. So the question is whether it's necessary to
> > open up a 72 hour window where the general dev team has a chance to review
> > the code, when presumably all of the people who care should be involved, if the
> > feature is progressing properly. I'm not entirely sure.
> > 
> [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours. Those who need to be involved should be engaged early on throughout the development. If we push MERGE request as the formal mechanism for the community to review and respond it may be too late and I doubt how much of that will happen even in 72 hours. 

I agree with the comments that it shouldn't be the only time for review
and discussion.

The more I observe people interacting with master, the more I'd respond
to this by asking:  Why are we ever in a rush to merge changes in?

Shouldn't community consensus and master stability be more important
than anything specific feature?

-chip

Re: Branch Merge Expectations - Draft for Discussion

Posted by Chip Childers <ch...@sungard.com>.
On Fri, Feb 22, 2013 at 02:17:58PM -0800, Alex Huang wrote:
> Also I like to add a note about DB upgrades must be completed before the merge.
> 
> --Alex

Added

RE: Branch Merge Expectations - Draft for Discussion

Posted by Sudha Ponnaganti <su...@citrix.com>.
+1

-----Original Message-----
From: Alex Huang [mailto:Alex.Huang@citrix.com] 
Sent: Friday, February 22, 2013 2:18 PM
To: cloudstack-dev@incubator.apache.org
Subject: RE: Branch Merge Expectations - Draft for Discussion

Also I like to add a note about DB upgrades must be completed before the merge.

--Alex

> -----Original Message-----
> From: dcahill@midokura.jp [mailto:dcahill@midokura.jp] On Behalf Of 
> Dave Cahill
> Sent: Thursday, February 21, 2013 5:41 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Branch Merge Expectations - Draft for Discussion
> 
> The one change I'd like here is to add a note about post-merge, as a 
> reminder to not "merge and run".
> 
> Testing comprehensively before merging should cover most issues, but I 
> think it's woth adding a note that you should plan to wait for Jenkins 
> builds to complete successfully before considering your work done.
> 
> On Fri, Feb 22, 2013 at 4:55 AM, Animesh Chaturvedi < 
> animesh.chaturvedi@citrix.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> > > Sent: Thursday, February 21, 2013 9:02 AM
> > > To: cloudstack-dev@incubator.apache.org
> > > Subject: Re: Branch Merge Expectations - Draft for Discussion
> > >
> > > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net>
> wrote:
> > > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> > > >> Do we really need to wait 72 hours for all merge requests? I 
> > > >> feel that slows developers down unless they plan very well.
> > > >
> > > > What's wrong with the expectation being that they plan very well?
> > > > ;-)
> > > >
> > > > Remember, "community over code." The point of waiting 72 hours 
> > > > is to give the community the opportunity to review, comment, etc.
> > > >
> > > > The point that some merges are less disruptive / intrusive than 
> > > > others is well-taken, though. Perhaps that is something that 
> > > > could be discussed during the feature proposal and decided then. 
> > > > If the community decides up-front that a merge is unlikely to be 
> > > > a problem, then maybe the expectation would be that only 48 or 
> > > > 24 hours needs to pass to allow for review & comments. But it 
> > > > should be explicit, and I'd rather err on the side of allowing 
> > > > the community
> time to review.
> > >
> > > I think the idea is that the people that a review would be 
> > > targeted at
> > are likely
> > > already involved, or perhaps review has been requested 
> > > independently
> > prior to
> > > formally requesting the merge. So the question is whether it's 
> > > necessary
> > to
> > > open up a 72 hour window where the general dev team has a chance 
> > > to
> > review
> > > the code, when presumably all of the people who care should be 
> > > involved,
> > if the
> > > feature is progressing properly. I'm not entirely sure.
> > >
> > [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours.
> > Those who need to be involved should be engaged early on throughout 
> > the development. If we push MERGE request as the formal mechanism 
> > for the community to review and respond it may be too late and I 
> > doubt how much of that will happen even in 72 hours.
> >
> > > >
> > > > Best,
> > > >
> > > > jzb
> > > > --
> > > > Joe Brockmeier
> > > > jzb@zonker.net
> > > > Twitter: @jzb
> > > > http://www.dissociatedpress.net/
> >

RE: Branch Merge Expectations - Draft for Discussion

Posted by Alex Huang <Al...@citrix.com>.
Also I like to add a note about DB upgrades must be completed before the merge.

--Alex

> -----Original Message-----
> From: dcahill@midokura.jp [mailto:dcahill@midokura.jp] On Behalf Of Dave
> Cahill
> Sent: Thursday, February 21, 2013 5:41 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Branch Merge Expectations - Draft for Discussion
> 
> The one change I'd like here is to add a note about post-merge, as a
> reminder to not "merge and run".
> 
> Testing comprehensively before merging should cover most issues, but I
> think it's woth adding a note that you should plan to wait for Jenkins builds to
> complete successfully before considering your work done.
> 
> On Fri, Feb 22, 2013 at 4:55 AM, Animesh Chaturvedi <
> animesh.chaturvedi@citrix.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> > > Sent: Thursday, February 21, 2013 9:02 AM
> > > To: cloudstack-dev@incubator.apache.org
> > > Subject: Re: Branch Merge Expectations - Draft for Discussion
> > >
> > > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net>
> wrote:
> > > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> > > >> Do we really need to wait 72 hours for all merge requests? I feel
> > > >> that slows developers down unless they plan very well.
> > > >
> > > > What's wrong with the expectation being that they plan very well?
> > > > ;-)
> > > >
> > > > Remember, "community over code." The point of waiting 72 hours is
> > > > to give the community the opportunity to review, comment, etc.
> > > >
> > > > The point that some merges are less disruptive / intrusive than
> > > > others is well-taken, though. Perhaps that is something that could
> > > > be discussed during the feature proposal and decided then. If the
> > > > community decides up-front that a merge is unlikely to be a
> > > > problem, then maybe the expectation would be that only 48 or 24
> > > > hours needs to pass to allow for review & comments. But it should
> > > > be explicit, and I'd rather err on the side of allowing the community
> time to review.
> > >
> > > I think the idea is that the people that a review would be targeted
> > > at
> > are likely
> > > already involved, or perhaps review has been requested independently
> > prior to
> > > formally requesting the merge. So the question is whether it's
> > > necessary
> > to
> > > open up a 72 hour window where the general dev team has a chance to
> > review
> > > the code, when presumably all of the people who care should be
> > > involved,
> > if the
> > > feature is progressing properly. I'm not entirely sure.
> > >
> > [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours.
> > Those who need to be involved should be engaged early on throughout
> > the development. If we push MERGE request as the formal mechanism for
> > the community to review and respond it may be too late and I doubt how
> > much of that will happen even in 72 hours.
> >
> > > >
> > > > Best,
> > > >
> > > > jzb
> > > > --
> > > > Joe Brockmeier
> > > > jzb@zonker.net
> > > > Twitter: @jzb
> > > > http://www.dissociatedpress.net/
> >

Re: Branch Merge Expectations - Draft for Discussion

Posted by Chip Childers <ch...@sungard.com>.
On Fri, Feb 22, 2013 at 10:40:38AM +0900, Dave Cahill wrote:
> The one change I'd like here is to add a note about post-merge, as a
> reminder to not "merge and run".
> 
> Testing comprehensively before merging should cover most issues, but I
> think it's woth adding a note that you should plan to wait for Jenkins
> builds to complete successfully before considering your work done.

+1 to that.  It's been quite difficult to work with master recently.
I've heard from several people (off list) that are quite
frustrated with the fact that master doesn't seem to be respected as a
shared tree that we should keep stable.

I'll update that page.

> 
> On Fri, Feb 22, 2013 at 4:55 AM, Animesh Chaturvedi <
> animesh.chaturvedi@citrix.com> wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> > > Sent: Thursday, February 21, 2013 9:02 AM
> > > To: cloudstack-dev@incubator.apache.org
> > > Subject: Re: Branch Merge Expectations - Draft for Discussion
> > >
> > > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
> > > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> > > >> Do we really need to wait 72 hours for all merge requests? I feel
> > > >> that slows developers down unless they plan very well.
> > > >
> > > > What's wrong with the expectation being that they plan very well? ;-)
> > > >
> > > > Remember, "community over code." The point of waiting 72 hours is to
> > > > give the community the opportunity to review, comment, etc.
> > > >
> > > > The point that some merges are less disruptive / intrusive than others
> > > > is well-taken, though. Perhaps that is something that could be
> > > > discussed during the feature proposal and decided then. If the
> > > > community decides up-front that a merge is unlikely to be a problem,
> > > > then maybe the expectation would be that only 48 or 24 hours needs to
> > > > pass to allow for review & comments. But it should be explicit, and
> > > > I'd rather err on the side of allowing the community time to review.
> > >
> > > I think the idea is that the people that a review would be targeted at
> > are likely
> > > already involved, or perhaps review has been requested independently
> > prior to
> > > formally requesting the merge. So the question is whether it's necessary
> > to
> > > open up a 72 hour window where the general dev team has a chance to
> > review
> > > the code, when presumably all of the people who care should be involved,
> > if the
> > > feature is progressing properly. I'm not entirely sure.
> > >
> > [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours.
> > Those who need to be involved should be engaged early on throughout the
> > development. If we push MERGE request as the formal mechanism for the
> > community to review and respond it may be too late and I doubt how much of
> > that will happen even in 72 hours.
> >
> > > >
> > > > Best,
> > > >
> > > > jzb
> > > > --
> > > > Joe Brockmeier
> > > > jzb@zonker.net
> > > > Twitter: @jzb
> > > > http://www.dissociatedpress.net/
> >

Re: Branch Merge Expectations - Draft for Discussion

Posted by Dave Cahill <dc...@midokura.com>.
The one change I'd like here is to add a note about post-merge, as a
reminder to not "merge and run".

Testing comprehensively before merging should cover most issues, but I
think it's woth adding a note that you should plan to wait for Jenkins
builds to complete successfully before considering your work done.

On Fri, Feb 22, 2013 at 4:55 AM, Animesh Chaturvedi <
animesh.chaturvedi@citrix.com> wrote:

>
>
> > -----Original Message-----
> > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> > Sent: Thursday, February 21, 2013 9:02 AM
> > To: cloudstack-dev@incubator.apache.org
> > Subject: Re: Branch Merge Expectations - Draft for Discussion
> >
> > On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
> > > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> > >> Do we really need to wait 72 hours for all merge requests? I feel
> > >> that slows developers down unless they plan very well.
> > >
> > > What's wrong with the expectation being that they plan very well? ;-)
> > >
> > > Remember, "community over code." The point of waiting 72 hours is to
> > > give the community the opportunity to review, comment, etc.
> > >
> > > The point that some merges are less disruptive / intrusive than others
> > > is well-taken, though. Perhaps that is something that could be
> > > discussed during the feature proposal and decided then. If the
> > > community decides up-front that a merge is unlikely to be a problem,
> > > then maybe the expectation would be that only 48 or 24 hours needs to
> > > pass to allow for review & comments. But it should be explicit, and
> > > I'd rather err on the side of allowing the community time to review.
> >
> > I think the idea is that the people that a review would be targeted at
> are likely
> > already involved, or perhaps review has been requested independently
> prior to
> > formally requesting the merge. So the question is whether it's necessary
> to
> > open up a 72 hour window where the general dev team has a chance to
> review
> > the code, when presumably all of the people who care should be involved,
> if the
> > feature is progressing properly. I'm not entirely sure.
> >
> [Animesh>] Marcus, thanks for clarifying my opinion is similar to yours.
> Those who need to be involved should be engaged early on throughout the
> development. If we push MERGE request as the formal mechanism for the
> community to review and respond it may be too late and I doubt how much of
> that will happen even in 72 hours.
>
> > >
> > > Best,
> > >
> > > jzb
> > > --
> > > Joe Brockmeier
> > > jzb@zonker.net
> > > Twitter: @jzb
> > > http://www.dissociatedpress.net/
>

RE: Branch Merge Expectations - Draft for Discussion

Posted by Animesh Chaturvedi <an...@citrix.com>.

> -----Original Message-----
> From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> Sent: Thursday, February 21, 2013 9:02 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: Branch Merge Expectations - Draft for Discussion
> 
> On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
> > On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> >> Do we really need to wait 72 hours for all merge requests? I feel
> >> that slows developers down unless they plan very well.
> >
> > What's wrong with the expectation being that they plan very well? ;-)
> >
> > Remember, "community over code." The point of waiting 72 hours is to
> > give the community the opportunity to review, comment, etc.
> >
> > The point that some merges are less disruptive / intrusive than others
> > is well-taken, though. Perhaps that is something that could be
> > discussed during the feature proposal and decided then. If the
> > community decides up-front that a merge is unlikely to be a problem,
> > then maybe the expectation would be that only 48 or 24 hours needs to
> > pass to allow for review & comments. But it should be explicit, and
> > I'd rather err on the side of allowing the community time to review.
> 
> I think the idea is that the people that a review would be targeted at are likely
> already involved, or perhaps review has been requested independently prior to
> formally requesting the merge. So the question is whether it's necessary to
> open up a 72 hour window where the general dev team has a chance to review
> the code, when presumably all of the people who care should be involved, if the
> feature is progressing properly. I'm not entirely sure.
> 
[Animesh>] Marcus, thanks for clarifying my opinion is similar to yours. Those who need to be involved should be engaged early on throughout the development. If we push MERGE request as the formal mechanism for the community to review and respond it may be too late and I doubt how much of that will happen even in 72 hours. 

> >
> > Best,
> >
> > jzb
> > --
> > Joe Brockmeier
> > jzb@zonker.net
> > Twitter: @jzb
> > http://www.dissociatedpress.net/

Re: Branch Merge Expectations - Draft for Discussion

Posted by Marcus Sorensen <sh...@gmail.com>.
On Thu, Feb 21, 2013 at 9:40 AM, Joe Brockmeier <jz...@zonker.net> wrote:
> On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
>> Do we really need to wait 72 hours for all merge requests? I feel that
>> slows developers down unless they plan very well.
>
> What's wrong with the expectation being that they plan very well? ;-)
>
> Remember, "community over code." The point of waiting 72 hours is to
> give the community the opportunity to review, comment, etc.
>
> The point that some merges are less disruptive / intrusive than others
> is well-taken, though. Perhaps that is something that could be discussed
> during the feature proposal and decided then. If the community decides
> up-front that a merge is unlikely to be a problem, then maybe the
> expectation would be that only 48 or 24 hours needs to pass to allow for
> review & comments. But it should be explicit, and I'd rather err on the
> side of allowing the community time to review.

I think the idea is that the people that a review would be targeted at
are likely already involved, or perhaps review has been requested
independently prior to formally requesting the merge. So the question
is whether it's necessary to open up a 72 hour window where the
general dev team has a chance to review the code, when presumably all
of the people who care should be involved, if the feature is
progressing properly. I'm not entirely sure.

>
> Best,
>
> jzb
> --
> Joe Brockmeier
> jzb@zonker.net
> Twitter: @jzb
> http://www.dissociatedpress.net/

Re: Branch Merge Expectations - Draft for Discussion

Posted by Joe Brockmeier <jz...@zonker.net>.
On Wed, Feb 20, 2013, at 04:23 PM, Animesh Chaturvedi wrote:
> Do we really need to wait 72 hours for all merge requests? I feel that
> slows developers down unless they plan very well. 

What's wrong with the expectation being that they plan very well? ;-)

Remember, "community over code." The point of waiting 72 hours is to
give the community the opportunity to review, comment, etc. 

The point that some merges are less disruptive / intrusive than others
is well-taken, though. Perhaps that is something that could be discussed
during the feature proposal and decided then. If the community decides
up-front that a merge is unlikely to be a problem, then maybe the
expectation would be that only 48 or 24 hours needs to pass to allow for
review & comments. But it should be explicit, and I'd rather err on the
side of allowing the community time to review.

Best,

jzb
-- 
Joe Brockmeier
jzb@zonker.net
Twitter: @jzb
http://www.dissociatedpress.net/

RE: Branch Merge Expectations - Draft for Discussion

Posted by Animesh Chaturvedi <an...@citrix.com>.
Chip

I posted this in another thread where 72 hour was called out,


Do we really need to wait 72 hours for all merge requests? I feel that slows developers down unless they plan very well. We had similar discussion just 3 weeks back and it was discussed that it depends on the scope and dependencies with other modules on case-by-case basis. If all discussion and development and review has happened by community and if it is isolated feature merge can be done quickly.

Here is link to that discussion.

http://mail-archives.apache.org/mod_mbox/incubator-cloudstack-dev/201301.mbox/%3C93099572B72EB341B81A644E134F240B012F747FE622@SJCPMAILBOX01.citrite.net%3E



> -----Original Message-----
> From: Chip Childers [mailto:chip.childers@sungard.com]
> Sent: Wednesday, February 20, 2013 2:04 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Branch Merge Expectations - Draft for Discussion
> 
> Hi all,
> 
> I spent some time working on a draft of "branch merge expectations" [1], and
> I'd really like to get feedback on it.
> 
> Did I document our consensus on the topic correctly?  Is everyone comfortable
> with these guidelines?  Did I miss anything?
> 
> -chip
> 
> [1]
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Exp
> ectations