The Problem
Bitbucket is great. I use it as often as possible both for my own private work and for collaborative efforts. It’s easy to use and has most of the features small teams are likely to need. One shortcoming however is it’s code review functionality. Back in 2012 Bitbucket introduced “lightweight” code reviews but when they say lightweight they really mean it. Essentially, code reviews are bound to to pull requests so your only opportunity to review code is when it is originally written or when it changes.
At a high level this seems reasonable; if all pull requests are reviewed then that means that every line of production code has been reviewed, approved and safe for release, right? The problem is that pull requests are generally small and homogenous, addressing one bug or new feature in isolation. They can look great in and of themselves while still being a bad fit for the overall design of the product or module to which they belong.
My favorite illustration of products that evolve this way has to be the one of the cat with the elephant trunk and the human hand sticking out of it’s back. When the pull request containing the “hand” functionality was being reviewed it probably made perfect sense. Same with the “elephant trunk” piece, especially to reviewers unfamiliar with the architect’s design goals and may not have known that the mammal being built was intended to resemble a cat. This sounds contrived I know, but it happens all the time. Just think back to the last time you found yourself reviewing code for a product on which you don’t typically work.
The argument could be made that the architect should be catching these kinds of issues but the reality is that it doesn’t always happen. Or maybe you just got a code drop from a contractor that insisted on developing in stealth mode up until the last minute and now you need to critique the whole thing. Whatever path got you there, sometimes you just need to review an entire product or module. There are tools to do this such as Atlassian’s Crucible and Codifferous but they may end up costing money (particularly if your repos are private) and may require you to run your own servers…not the ideal scenario for private work or small, unfunded collaborations.
The Solution
Here’s a workflow that allows any file in a Bitbucket repository to be reviewed as part of a single code review and requires no extra tools:
- Architect (or repo owner, etc.) branches the repository of interest at the desired revision and name it something like FullCodeReview_07_10_2014.
- Architect checks out the new branch and defines the scope of the review by adding comments to the source files. I’d suggest formatting the comments like this:
// CREVIEW <Reviewer's Initials>: <Reviewer's comment(s)>
(The utility of the CREVIEW prefix becomes obvious in step 8)
- Architect pushes changes back into the remote repo and issues a Pull Request back to whatever branch the code review branch was created off of, adding code review participants to that Pull Request.
- Participants either check out the code and add comments to new source files or sections (if they want to increase the scope of the code review) or add comments on the sections tagged via comments by the Architect directly to the Pull Request using Bitbucket’s built in code review tools.
- Participants push changes made to the source back to the remote repo for FullCodeReview_07_10_2014. Changes should generally consist only of CREVIEW comments but could also include actual code changes if the goal of the review is to make changes as you go.
- Repeat steps 4-5 as many times as necessary to conclude the review.
- If the goal of the code review was to generate work, create bugs, stories or whatever your team uses to track work to be done and then either tag and delete the code review branch or if you aren’t worried about branch clutter leave the branch there and go back to working on another branch. If you don’t care about preserving the history of the code review you could also just delete the FullCodeReview_07_10_2014 branch.
- If the final state of the code review branch includes changes that should be kept, the Architect should first remove all CREVIEW comments (this should be very easy to do since they all share the same unique prefix) then commit and then merge the original Pull Request. At this point it should be safe to just delete the FullCodeReview_07_10_2014 branch.
There is a caveat; this workflow will not work across multiple repositories…a limitation that includes submodules. In practice this is not typically a problem but it’s still worth pointing out.