On May 26, 2010, at 5:41 AM, Laurens Van Houtven wrote:

> Major stuff could be a blueprint on Launchpad. Blueprints match a branch for 
> the "big feature". So, we have the Twisted blueprint 
> quantum-transmogrification and a branch 
> lp:~lvh/twisted/quantum-transmogrification.

So, while I can definitely sympathize with a certain animosity towards trac, 
and I can appreciate the goals and sensibilities of launchpad, I will probably 
flat-out veto any required / process-driven usage of Launchpad blueprints.  
Bugs, features, enhancements, etc, are all units of work that need to be 
tracked, and it's better to have one kind of crummy interface for tracking 
_everything_ than three interfaces, even three good interfaces, for tracking 
little bits stuff in different ways according to arbitrary distinctions.  (As 
someone recently opined to me, Blueprints are a giant complicated interface for 
pasting the URL to a Google Wave into a text field.  We might as well skip the 
text field and just link straight to the conversation from a Trac ticket.)

> From that branch I create a bunch of branches of review units (if it turns 
> out it's too big, I just branch again for a new review unit). So, I want to 
> do something with entanglement: 
> lp:~lvh/twisted/quantum-transmogrification-entanglement, and it's good, so 
> someone reviews and sends it back.
> 
> lp's merge proposals let you do the code review in arbitrarily small chunks. 
> So if the thing I do next is 
> lp:~lvh/twisted/quantum-transmogrification-ftl-travel,

lp:~lvh... isn't a verb.  What do you do with that string? :)

> and it turns out FTL travel is really really hard so I need two smaller 
> branches lp:~lvh/twisted/quantum-transmogrification-tunnels and 
> lp:~lvh/twisted/quantum-transmogrification-ansible. Both are good, so they 
> get put back into ~lvh/twisted/quantum-transmogrification-ftl-travel.
> 
> Each review would verify that all children (if any) have also been reviewed. 
> So, the final review is pretty small, as suggested :-)


The review wouldn't verify that the parent had been reviewed, though.  If you 
started this process by writing a bunch of code in the q-t-f-t branch, *that* 
code would never have been reviewed; unless q-t-f-t needs to be reviewed in its 
entirety before landing on trunk.  Which is precisely what I'm trying to avoid.

> This does not limit a developer's freedom to branch at will, because code 
> review is opt-in (merge proposal), not opt-out. If you don't do it, that code 
> in that branch isn't covered by a previous review, and must be reviewed later.

This strikes me as placing a pretty nasty burden on the reviewer.  The reviewer 
has to figure out if there are any commits that went only to the integration 
branch, isolate them, review them, get that branch into an OK-it's-reviewed 
state, while meanwhile other developers might be committing stuff to that 
branch and changing its contents, both from regular working commits and from 
reviewed merges.  It sounds like a nightmare.  Maybe bzr makes it easier than 
it sounds, but it sounds bad enough that even a big improvement would still be 
pretty bad ;).

> How exactly code review coverage would work is somewhat of an open question 
> and it's the obvious failure in this system. We use it in production and it 
> turns out to not be a problem, because people always end up doing two things:

Who is "we"?  What is "production"?  Are you talking about Twisted or a 
hypothetical project which uses Twisted, or a fork of Twisted on Launchpad?  Is 
this a hypothetical project or a real proejct?  I am super confused.

> 1) always branch at least once from the first branch off trunk (so branch off 
> lp:~lvh/twisted/quantum-transmogrification). Net result: 
> lp:~lvh/twisted/quantum-transmogrification only introduces code in the form 
> of merges.

That's pretty much what I'm proposing, except I don't actually care whether 
they're merges or patches or individual commits, as long as they've cycled 
through code-review properly.

> 2) always do code review on branches being merged into your first branch off 
> trunk (so everything merged into lp:~lvh/twisted/quantum-transmogrification 
> has to be reviewed already)

And this is what we already do.

> Note that our merges into trunk are automagic.

(Again, who is "we", and by what mechanism are they automated?  Are you 
proposing that we do this, or are you stating that some other people do?)

> If it's merged into a direct branch off of trunk and it satisfies some 
> qualities (such as full test coverage :)), it gets put into trunk, and that 
> gets pushed to production servers. No human interaction. Scary at first, but 
> then you realize humans were already involved in the QC process extensively 
> at every point -- doing it this way just makes them take testing more 
> seriously :)

Human interaction of some kind should definitely be required for Twisted.  This 
is not just pushing some new widget to a web site; this is potentially pushing 
out new APIs that need to be documented and supported to a whole ton of 
developers.  The whole point of the process modification I've proposed is to 
make sure that features arrive in releases as coherent, comprehensible whole 
pieces, not to allow things we can automatically verify (like docstring and 
test coverage) to be deferred to later merges.  These properties of the code 
should still be verified on every merge to the integration branch; the 
interesting thing about the merge to trunk is the verification that the unit is 
a coherent whole (and in the case of a deprecation / replacement, that the 
replacement is a functionally adequate upgrade).

> I think a bug would be similar except the root would not be a blueprint but a 
> bug.

So, I'm really confused as to what the purpose of this message was - are you 
just describing how a similar workflow might work if we used launchpad, 
advocating that we switch to launchpad in order to implement this, advocating 
that we use launchpad for big features but *not* for other stuff, or ... what?  
If you're proposing a different functional modification to the existing 
process, can you do it without reference to tons of launchpad-specific 
terminology?

Sorry if this comes off as a little flamey; I really am just confused as to 
what the point was.

Thanks,

-glyph



_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to