incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus Sorensen <shadow...@gmail.com>
Subject Re: Branch Merge Expectations - Draft for Discussion
Date Fri, 22 Feb 2013 17:15:54 GMT
On Fri, Feb 22, 2013 at 8:42 AM, Chip Childers
<chip.childers@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 <jzb@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

Mime
View raw message