How to create a multi-patch review in reviewboard

August 6, 2012 — How to create a multi-patch review in reviewboard

 

    Reviewboard (http://www.reviewboard.org/) is web based software to make code reviews easier.  It works in conjunction with svn ( http://subversion.tigris.org/ ) which is some revision control software.  They also claim to support git, but they are full of shit, or do not have a clue how git is actually used.

The main problem I have with reviewboard is that if you are working on some feature, and the implementaton of that feature consists of several logical changes to the code which you would like to review separately, but which need to be committed together to make a useful feature, reviewboard has no support for such a sane workflow. I also like to “polish” the patches prior to committing, so that when looking back over the history, it looks nice and clean with each commit being as much as possible a nice, complete unit of work. This makes later cherry picking for other branches, or porting to other codebases (e.g. porting driver changes from the kernel.org kernel back to whatever kernel redhat or SuSE is using today) much easier.

As an example, before adding some feature, I might want to factor out some specialized code to make it more general to support code which is part of this new feature.  I want the factoring out to be a separate commit from the feature code, but the feature code requires the factoring out code to precede it (or even make a diff to review.)  Let’s call the factoring out patch “A” and the feature patch “B”.  Now imagine instead of 2 patches, you have 26 such patches (A thru Z) with dependencies like this.  Reviewboard has no sensible way to even begin reviewing patch “B” until “A” is committed to the repository.  Likewise, “C” cannot begin the review process until “B” is committed.  This is clearly insane.  But the reviewboard developers do not appear to see it this way (I don’t think they even perceive that there is a problem, and seem to think (or think that I think) that quilt and stacked git are “revision control” systems (scorn quotes are theirs).  See: this bug report. ) (Edit: perusing comments here and here makes me think at least some of the reviewboard developers are cognizant of the issue I’m having, despite my bug report being summarily shot down. Don’t know if they plan to do anything about it any time soon though.)

After struggling with this problem for awhile I came up with a workaround.  Sort of.  You still have to commit “A” before “B” can be reviewed, and “B” before “C”, etc.  The trick is you commit them to a branch created solely to satisfy the demands of reviewboard, which is used only by reviewboard and for no other purpose. Which isn’t much of a trick really, it only took me so long to come up with because it’s such a dumb, brute-force sledgehammer approach. For large repositories it might not be practical.

How to create multi patch reviews in reviewboard:

  1. “svn cp” the base code into a special branch just for this review. , for example:

    To shorten things, let’s create some shell variables:

    • export REPO=svnhost/svnrepo/yourproject/
    • export DEVBRANCH=branches/normal-development-branch
    • export REVIEWDIR=yourproject-todaysdate-review
    • export RBBRANCH=branches/reviewboard/$REVIEWDIR

    svn –username=you cp https://$REPO/$DEVBRANCH https://$REPO/$RBBRANCH

  2. Check out your review branch:

    svn –username=you co https://$REPO/$RBBRANCH $REVIEWDIR

    cd $REVIEWDIR

  3. Import your patches, then pop them all off

    (using git and stacked git ):

    git init
    git add .
    git commit -m 'initial commit'
    git branch mybranch
    git checkout mybranch
    stg init
    stg import -s ~/my-patches/series
    stg pop -a
    
  4. For each patch in the series:
    1. Push the patch and make a svn diff of it.
      stg push
      svn diff > /tmp/my-patch-xxx 
      # use something unique for xxx, like increasingnumbers or something.
      
      
    2. Make a reviewboard review request.
      • Use $RBBRANCH as the “base”
      • Upload the diff my-patch-xxx
      • cut and paste stuff from “stg show” output into reviewboard webpage for summary and description, and fill out all the usual reviewboard stuff.
    3. Commit to svn (but only to the fake reviewboard branch specially created for this review)

      ~ by scaryreasoner on August 7, 2012.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

 
%d bloggers like this: