Re: [Monotone-devel] code review in practice

[Evan Martin]

On 4/10/07, Caio Marcelo <address@hidden> wrote:
> Hello Evan and people,
>
> Recently (after re-reading your blog post about this subject) I wrote

It's always a little frustrating when the only Google results I can
find on a subject (in the case, the query [monotone code review] is
something I've written myself)...

> http://www.students.ic.unicamp.br/~ra015599/PLAN
>
> maybe you'll find some of these helpful. After writing this I thought
> maybe the web interface could be an optional layer over the "offline
> compatible" script around Monotone. Comments on that PLAN are very
> welcome.

Regarding codereview branch names: rather than being careful about
naming different branches (and without using a sequence number), it's
more in the monotone style to just name the branch after a sha-1.  For
example, you could take the revision id itself as the branch name.

I agree that a web interface is a somewhat separate task from the
problem of reviewing.  For example, without integrating into an email
interface or web stuff at all, a basic script that dumps to my console
"your commit is branch name foo.codereview.a78fd" would be enough for
me to send that info to a mailing list.  Then the approver could run
something like "mtn-codereview i-approve foo.codereview.a78fd" once
they like it.  (I guess it's maybe a bit more complicated than that
because you really want a code reviewer to approve a *revision*, not a
branch, as the latter could have more changes come along after the
reviewer looked at it...)

> On 4/10/07, Evan Martin <address@hidden> wrote:
> > 2) Submitters make a (named) branch for each new change, branch only
> > merged into the main branch after it's been ok'd.
> > Analysis: a new branch name for every change seems a little scary.
>
> I choose this path, but with some "separate" namespace for the new
> changes and an automatic way to assign this names. After a few commits
> (and reviews), reviewer is happy and just mtn approve the head of the
> 'review branch' into the main branch. AFAIK, if you just use main
> branch you will get access ONLY to revisions approved by someone.

Seeing your writeup of it makes me think using separate names isn't so
bad after all.

> (PS: thanks for the nice tech blog -- lots of good posts! evan++)

Thanks!  LJ doesn't provide any reader stats so I'm always surprised
to learn people read it.  :)