incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ewan Mellor <Ewan.Mel...@eu.citrix.com>
Subject RE: where features are developed was: Review Request: Merge Kelven's VPC code for Vmware into asf vpc branch
Date Mon, 06 Aug 2012 03:17:04 GMT
> -----Original Message-----
> From: Brett Porter [mailto:brett@porterclan.net] On Behalf Of Brett Porter
> Sent: 05 August 2012 05:35
> To: cloudstack-dev@incubator.apache.org
> Cc: cloudstack-dev@incubator.apache.org
> Subject: Re: where features are developed was: Review Request: Merge
> Kelven's VPC code for Vmware into asf vpc branch
> 
> 
> 
> On 04/08/2012, at 12:16 PM, Ewan Mellor <Ewan.Mellor@eu.citrix.com>
> wrote:
> 
> >> What happens if there's a rockstar NetScaler developer lurking here -
> >> how do they get involved in that? What if the people buddy-coding are
> >> working on the same feature as someone that doesn't work with them -
> >> how will they collaborate?
> >
> > My point wasn't that people won't be collaborating.  My point was that by
> the time things land in master, they may have multiple authors associated
> with them.  The people working on this branch have been prototyping,
> making mistakes along the way, and keeping up with changes in the systems
> that they're talking to.  And while all that has been going on, master has been
> moving forward at a great lick.
> 
> But where will they collaborate, in an inclusive manner, if not in the ASF repo
> and list, with the usual project patch submission mechanism?

They'll be collaborating in private branches.  Most people working on CloudStack aren't committers,
so this will be the only way it can work.  Your proposal implies that either:
1. Developers can't commit regularly and often (bad),
2. Non-committers are allowed to commit to feature branches (currently not allowed), or
3. We have committers review patches to unfinished feature branches as well as master (obviously
not workable).

Are you proposing that we change one of those?

> >
> > By the time the feature lands in master, we're not going to see all those
> prototypes and mistakes, because it would be impossible to merge them all
> by now, and we certainly don't want to review changes that are wrong.
> We're going to see a small number of clean changesets that apply against
> HEAD.  That's what we will be reviewing for submission into master.
> 
> That's part of the problem I see with this - the thought processes, prototypes
> and even mistakes are helpful for others to understand why something
> ended up the way it did. Early review is far more effective than reviewing a
> large final commit if you have suggestions, especially if they are fundamental
> to the change. All that information is useful to keep in the ASF repository
> history.

It's arguable whether that history is useful to anyone, but anyway that's not the point. 
The issue is whether people have to perform peer-review on those mistakes.  The mistakes might
be useful for historical reasons, but they certainly aren't something that we want our committers
to be peer-reviewing.  By the time it's ready for review, those changesets have to be collapsed
into a logical, consistent, and mistake-free set, otherwise they're impossible to review.

> > The consequence of that is that a single changeset will have multiple
> authors.  This is what happened to Vijay B the other day -- he turned up with
> a sanitized, ready-for-merge changeset, and he got shouted at for submitting
> someone else's code.  We have to figure out how to handle this.  Telling him
> to go away isn't going to work.
> 
> Let's be clear - no one is being shouted at, and no one is being told to go
> away - quite the opposite. I certainly apologise if I've come off that way.

No, you weren't coming off that way at all.  Other people on this list were, though.

Cheers,

Ewan.


Mime
View raw message