No subject

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, 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 :-) 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.

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:

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.
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)

Note that our merges into trunk are automagic. 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 :)

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


Laurens

--001485f44be061c57f04877c13a5
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

I've said something in #twisted but I hadn't read this reply yet, s=
o for sake of saving this for posterity, I agree with jml here. I think we&=
#39;re mostly being bitten because of a lack of software tools, in the form=
 of svn and trac. Disclaimer: I really dislike svn since I never figured ou=
t how Combinator works. I really dislike trac.<div>
<br></div><div>So, for the rest of this e-mail, let&#39;s pretend we&#39;re=
 implementing a big new feature since that&#39;s the thing I tried to do to=
 some extent.</div><div><br></div><div>Major stuff could be a blueprint on =
Launchpad. Blueprints match a branch for the &quot;big feature&quot;. So, w=
e have the Twisted blueprint quantum-transmogrification and a branch lp:~lv=
h/twisted/quantum-transmogrification.</div>
<div><br></div><div>From that branch I create a bunch of branches of review=
 units (if it turns out it&#39;s too big, I just branch again for a new rev=
iew unit). So, I want to do something with entanglement: lp:~lvh/twisted/qu=
antum-transmogrification-entanglement, and it&#39;s good, so someone review=
s and sends it back.</div>
<div><br></div><div>lp&#39;s merge proposals let you do the code review in =
arbitrarily small chunks. So if the thing I do next is=C2=A0lp:~lvh/twisted=
/quantum-transmogrification-ftl-travel, and it turns out FTL travel is real=
ly really hard so I need two smaller branches=C2=A0lp:~lvh/twisted/quantum-=
transmogrification-tunnels and=C2=A0lp:~lvh/twisted/quantum-transmogrificat=
ion-ansible. Both are good, so they get put back into=C2=A0~lvh/twisted/qua=
ntum-transmogrification-ftl-travel.</div>
<div><br></div><div>Each review would verify that all children (if any) hav=
e also been reviewed.=C2=A0So, the final review is pretty small, as suggest=
ed :-) This does not limit a developer&#39;s freedom to branch at will, bec=
ause code review is opt-in (merge proposal), not opt-out. If you don&#39;t =
do it, that code in that branch isn&#39;t covered by a previous review, and=
 must be reviewed later.</div>
<div><br></div><div>How exactly code review coverage would work is somewhat=
 of an open question and it&#39;s the obvious failure in this system. We us=
e it in production and it turns out to not be a problem, because people alw=
ays end up doing two things:</div>
<div><br></div><div>1) always branch at least once from the first branch of=
f trunk (so branch off=C2=A0lp:~lvh/twisted/quantum-transmogrification). Ne=
t result:=C2=A0lp:~lvh/twisted/quantum-transmogrification only introduces c=
ode in the form of merges.</div>
<div>2) always do code review on branches being merged into your first bran=
ch off trunk (so everything merged into lp:~lvh/twisted/quantum-transmogrif=
ication has to be reviewed already)</div><div><br></div><div>Note that our =
merges into trunk are automagic. If it&#39;s merged into a direct branch of=
f 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 hum=
an interaction. Scary at first, but then you=C2=A0realize=C2=A0humans were =
already involved in the QC process extensively at every point -- doing it t=
his way just makes them take testing more seriously :)</div>
<div><br></div><div>I think a bug would be similar except the root would no=
t be a blueprint but a bug.</div><div><br></div><div><br></div><div>Laurens=
</div>

--001485f44be061c57f04877c13a5--