Update: We’ve documented this workflow in the RBTools documentation. The following still applies, but for more details and tips, see the docs.
One of the beautiful things about Git is that you have so many ways of making it work for you. This is also one of the frightening things about Git, particularly if you’re just starting out. There’s loads of documentation and blog posts covering all the ways you can use Git to manage your code or shoot yourself in the foot.
A question we’re often asked is how Git is supposed to be used with Review Board or RBCommons.
“How should I post changes,” they ask. “How should I land them?”
“Well,” we say, “that’s up to you… but here’s how we do it.”
One branch per review request
Branches in Git are pretty great. They’re light-weight, and you can really choose when and how to use them.
What we like to do is have one branch for every review request we’re still working with. Maybe they’re branching off of master, or maybe off of another change you have up for review… doesn’t matter.
Create the branch, and create as many commits on it as you want. You’re going to post these all for review under one review request. For our example, we’ll use 2 commits.
$ git checkout -b my-branch-1 master
$ vim foo.py
$ git commit -a
$ vim bar.py
$ git commit -a
Now let’s create another branch off of that, and make one commit here. This will be for your second review request.
$ git checkout -b my-branch-2
$ vim foo.py
$ git commit -a
Your tree now looks like this:
o [my-branch-2]
|
o [my-branch-1]
|
o
|
o [master] [origin/master]
|
Great, let’s post!
We’ll post that first change for review (my-branch-1). Since it’s based off of origin/master, this will be easy (since by default, that’s what’s diffed against). We just post like so:
$ git checkout my-branch-1
$ rbt post
Review request #1001 posted.
https://reviewboard.example.com/r/1001/
https://reviewboard.example.com/r/1001/diff/
Excellent. If you go to that first URL, you’ll see your summary and description filled in from your commit messages. You can edit these to your liking.
If your server has any default reviewers set up, they’ll be assigned. You might also want to fill in some bug, add some testing information. Do whatever you want to do there and publish the review request.
Now sit back and relax and… oh wait, you have a second change ready for review! Thanks to Git and RBTools, you don’t have to wait on that. Let’s post that one too.
$ rbt post my-branch-1..my-branch-2
Review request #1002 posted.
https://reviewboard.example.com/r/1002/
https://reviewboard.example.com/r/1002/diff/
What you’re doing here is posting all the commits on my-branch-2 that were made since my-branch-1. No need to push my-branch-1 first, or really worry about it in any way.
You’ll probably want to set the Depends On field to point to your other review request, as a hint to any reviewers deciding which to review first.
Oh, here’s some short-hand. If you’re already on my-branch-2, you can make use of HEAD instead of spelling out my-branch-2. In this case, this branch only has one commit, so you could also leave out my-branch1... All of these are therefore equivalent:
$ rbt post HEAD
$ rbt post my-branch-1..HEAD
$ rbt post my-branch-1..my-branch-2
This is probably familiar to you if you’re used to Git. You can use any Git SHA/tag/branch/revision range you want when calling rbt post.
Note: If you’re posting against a remote branch other than origin/master, you’ll need to either pass --tracking-branch=myremote/mybranch on any RBTools command, or set TRACKING_BRANCH = "myremote/mybranch" in .reviewboardrc. The remote must match the configured repository on Review Board.
Need to make some changes? -u to the rescue!
So someone found a flaw in your otherwise perfect code. Happens to the best of us. In both review requests, you say? Okay, we’ll let that slide for now.
Let’s update the first change. Lots of options here. You can make a new commit with the fixes, or you can amend the commit.
If it’s just a fix made in a previously un-pushed commit, we like to amend. Your choice.
$ git checkout my-branch-1
$ vim bar.py
$ git commit -a --amend
$ rbt post -u
Review request #1001 posted.
https://reviewboard.example.com/r/1001/
https://reviewboard.example.com/r/1001/diff/
Now on to the second. We’ll probably want the latest from my-branch-1 as well, so we can rebase or merge. We like to rebase when this stuff is still in flux and not yet pushed, and we like to merge when the history starts to matter (that is, when the code is in some kind of decent, landable shape).
Again, your call.
$ git checkout my-branch-2
$ git rebase my-branch-1
$ vim foo.py
$ git commit -a --amend
$ rbt post -u HEAD
Review request #1002 posted.
https://reviewboard.example.com/r/1002/
https://reviewboard.example.com/r/1002/diff/
The -u flag updates an existing review request that matches your commit message. If you’ve modified the summary or description in any way, it may prompt you for any review requests that mostly match. Just say yes or no.
Great, publish those changes. Eventually the code will be perfect.
Got your “Ship It!”? Time to land!
RBTools 0.7 and higher comes with a nifty little command, rbt land. This command takes a branch, verifies that it’s been reviewed, and lands the changes.
Let’s land both of your branches, one after the other.
$ git checkout master
$ rbt land --dest=master --push my-branch-1
$ rbt land --dest=master --push my-branch-2
This will verify that my-branch-1 is approved (at least one “Ship It!” and no open issues). It will then merge my-branch-1 into master, push it, and delete the old branch. Then it’ll verify, merge, push, and delete my-branch-2.
Each branch you land will be merged into master, with a merge commit containing the summary, description, bug numbers, and review request URL. If you want to instead squash each branch into a single commit on master, you can use --squash.
You can use --dry-run to see what will happen without actually changing your tree. Useful when you first start off.
You can also edit the commit message using --edit, or leave out --push if you don’t want to push the branch, or add --no-delete-branch if you don’t want to delete the branches. You can also set the default branch to land into. The documentation goes into all the options that are available.
Closing out landed review requests
We like to set up our review requests to auto-close when pushing commits. This is designed to work with rbt land.
When you land a change, the commit message will contain a line saying something like:
Reviewed at https://reviewboard.example.com/r/1001/
The auto-close hooks will see that and automatically close your review request, so you don’t have to.
And that’s how we do it.
There’s really a lot of options here. Some people push changes and then use the web UI to post them for review. Some people generate their own diffs and upload them. Some like to merge their own branches.
That’s all a lot of work, though. Our method give us:
- Nice code organization, since every review request has its own dedicated branch.
- Fast posting and updating of review requests.
- Less mess. No extra branches sticking around, and review requests are automatically closed.
- Confidence that every landed change has been approved. No slip-ups with pushing the wrong branch.
Give it a try!
Read More
