[opensource-dev] code review in the new release process

[Oz Linden (Scott Lawrence)]

On 2013-05-22 22:59 , Nicky Perian asks:
> Which repositories will be used to open code review? Will each project 
> be available for code review?

An excellent pair of questions... I'll take them in reverse order.

*How will you know which repositories are active, and will they be 
reviewable?*

    Our intent is that sources for development projects will become
    publicly visible /approximately/ when both:

      * A public test viewer is made available (whether as a Project or
        Beta viewer)
      * The design, especially any server interactions, is believed to
        be reasonably stable
        (if the goal of releasing a viewer is to find out whether or not
        a particular design works, we don't want to put the sources out
        where someone might pull them because then important changes to
        the design may create compatibility problems; the materials
        project kept its sources private for some time for this reason).

    I say approximately because each project will make that decision
    independently; for some, making sources public may be a high
    priority (such as getting important bug fixes out where others can
    pick them up), while others may take longer.  This is a guideline
    for our teams, not a hard and fast promise, but we are very much
    aware that it's in our interest to get new features adopted by the
    open source community in a timely way, so we have ample motivation
    to make sources public.

    To that end, I will be maintaining a wiki page that will display all
    of our Viewer channels and the latest viewers in the channel.
    Channels that include candidate viewer cohorts (normally only the
    release channel) will display both the default viewer and the
    candidates.  For each viewer, there will be a link to the
    repository, the changeset id it was built from, and an indicator as
    to whether or not that repository is public.   I've already created
    the program to generate this page content, and will try to update
    the page promptly when changes are made.  The page will appear
    shortly after we begin using the new viewer version management
    system (the generation program relies on queries against that
    service).  Incidentally - this will be separate from the
    user-oriented official Alternate Viewers page, which will provide
    the download links for each publicly available viewer.

    Bitbucket provides a 'watch' feature you can use to be notified when
    changes are made to repository - you can use that to monitor both
    viewer-release and any other repository, so I won't be configuring
    email notices on any of the new repositories.

*Which brings us to code reviews...*

The ReviewBoard instance at codereview.secondlife.com has been valuable, 
I think, but it has some significant problems - specifically it:

  * Isn't integrated with Jira
  * Isn't integrated with Bitbucket
  * Requires fairly complex manipulation to post reviews of code in
    repositories not directly descended from one of the configured repos
    (see Posting Failure
    <https://wiki.secondlife.com/wiki/Code_Review_Tool#Posting_Failure>
    in the wiki documentation)
    This goes directly to Nickys question, really, and I am not crazy
    about the idea of constantly having to configure each project
    repository... if only because it would get cluttered very quickly
  * Is rather a pain for me to keep up to date
    (updates require that I re-merge changes we need that for some
    reason the developers have never integrated my contributions for)...
    we're actually pretty far behind the most recent releases as a result.

Since I set up that review system, Bitbucket has significantly improved 
their display of differences in a commit and added some code review 
features (I like to think that my quite detailed feedback to them on 
this played some part in that).  If you display the page for a specific 
commit, there is a way to comment on both the commit as a whole (a box 
at the top) and on any specific line (click on the speech bubble to the 
left of the line).  The comments are then both mailed to the author of 
the commit and displayed on the page.  There's a way to reply to each, 
and there's an Approve button at the top that registers your approval of 
the commit.

Using Bitbucket for reviews would place a premium on arranging your 
changes as a single commit that does not have any embedded merges. As a 
former/sometime git user, I prefer to do that anyway.  It's a little 
less convenient in Mercurial, but it's not that hard.

On the whole, I think that using the Bitbucket review system would be 
much easier than the existing ReviewBoard; it seems to me that the only 
significant disadvantage is that it doesn't post review requests to this 
list, but putting together an email with a link doesn't seem to me to be 
too much to ask.

Opinions?  Experiments?

-- 
*Scott Lawrence* | /Director of Open Development/
Skype ozlinden <skype://ozlinden> | Second Life Oz Linden 
<https://my.secondlife.com/oz.linden>

Linden Lab| Makers of Shared Creative Spaces <http://lindenlab.com/>
SECOND LIFE <http://secondlife.com/> | PATTERNS 
<http://buildpatterns.com/> | CREATORVERSE <http://creatorverse.com/> | 
DIO <http://dio.com/> | VERSU <http://versu.com/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20130524/d682c15d/attachment-0001.htm 
-------------- next part --------------
An embedded message was scrubbed...
From: "Lirusaito" <commits-reply at bitbucket.org>
Subject: Re: [Bitbucket] Commit e6292c4: CHOP-942: fix crash if update check times
 out (lindenlab/viewer-3.5.3)
Date: Fri, 24 May 2013 15:00:48 -0000
Size: 8761
Url: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20130524/d682c15d/attachment-0002.eml 
-------------- next part --------------
An embedded message was scrubbed...
From: "Lirusaito" <commits-reply at bitbucket.org>
Subject: Re: [Bitbucket] Commit e6292c4: CHOP-942: fix crash if update check times
 out (lindenlab/viewer-3.5.3)
Date: Fri, 24 May 2013 15:01:08 -0000
Size: 16887
Url: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20130524/d682c15d/attachment-0003.eml