I review a lot of code on GitHub. It’s become more and more obvious that the code review interface and workflow on GitHub is really inadequate and annoying once you move beyond small patches and start looking at larger pieces of code. Here’s a mini rant and a description of what I do get around that in the hope that maybe someone has a better idea…
Problems
Here are the things that annoy me with GitHub on a daily basis:
It only shows me the latest diff, that is the diff between current master and the complete set of commits in the change request. Yes, I can tweak it to give me a diff between two arbitrary commits, but not if the PR is a single commit which was rebased with some new changes on it, and I can’t actually comment from the arbitrary diff view anyway. I can open a given commit and comment on that, but it doesn’t get added to the PR discussion flow and good luck finding those comments later…
It has no way to keep track of what I’ve reviewed and what I haven’t. There’s a notification thing that lights up when something happens on the pull request at large, but just opening the PR page clears the notification and now I have to remember by myself that there is stills tuff to look at. There’s no way to have it remember that I’ve reviewed the first two commits of five, or the first two files. I have to look through it all again and see if I recognize it or see something new. If I didn’t review everything in one fell swoop, I need to remember what I reviewed and what I didn’t.
All of this would be solved if there was a checkbox per commit and per file that meant “I have reviewed this”, then the default diff view would show the diff against what I’ve already signed off on.
(Normal bug reports also suffer from this, in that once you’ve glanced at the issue it’s completely off your radar until something happens on it again. I could add a tag or something for needs-another-look, but then every needs their own such tag and it just sucks…)
Solutions?
I’m not the only one to think this sucks. People have written bunches of tools to work around it! Here are some I tried:
-
gerrithub.io. It aims to integrate Gerrit with GitHub. Gerrit is very powerful, a lot of people like, and I frankly hate it. It’s built around the concept that a pull request (or the Gerrit equivalent) is a single commit only, and using it requires that all changes be pushed through Gerrit rather than through GitHub. Fail.
-
Review.Ninja. Looks somewhat promising. Integrates reasonably with GitHub but in the end doesn’t help with reviewing just the latest changes on rebases or reviewing files in separate sessions. Fail.
-
Reviewable.io. Does handle both of those issues, but can’t create line comments on diffs. Instead it generates a huge comment directly in the PR with lots of links to lines of source code and comments on them, and a bunch of pictures and unneccesary stats and stuff. I tried it out for a couple of days and the reactions from people seeing it from the GitHub perspective was mostly “WTF?”. Fail.
-
Codereviewhub.com. Upon adding the project to codereviewhub, it adds a huge blob of boilerplate crap to the PR description on every open PR. I gave up at that point, after manually cleaning the open pull requests of this mess… Fail.
-
Gitcolony.com. Seems somewhat promising, but failed to actually sync and load any commits from the open pull requests so I couldn’t actually try it out. Fail.
-
Phabricator. No GitHub integration. Fail. (I looked at it anyway because it wasn’t obvious to me from the outside that this was the case.)
These are the ones I found and could try out after lot of googling on things like “github code review replacement”. Some of them are probably perfectly fine if you’re a small team all using the tool. We have lots of casual and “drive by” contributions and I think we want to keep things close to the standard GitHub workflow for those people.
I know that when doing your first contribution to a new project, the last thing you want to do is learn some odd and arcane change management system just to get someone to look at your code. Sticking with the GitHub workflow here is the lowest common denominator.
What I Do Instead
Well, first of all, if it’s a twenty line diff in a single commit, I just do it on GitHub. It works perfectly fine.
But after not finding any actual tools out there that help (well enough) with bigger stuff, I just do it manually. I have a git alias git pr 1234 that checks out the pull request in question to a local branch and switches to it. I stare at the code in my editor. I look at the diffs. The diff view in the editor is quite nice. I have the PR open on GitHub in parallell and enter any actual comments there. As I’m done with files, I stage them using git add as usual. When I’m tired of reviewing or done for the moment, I create a branch reviewed-1234 and commit the stuff I have reviewed there. That creates a “bookmark” of where I was.
When something has changed, whether it be new commits or a rebase or whatever, I do a git pr 1234 again and repeat the process. But this time I also have the possibility of git d -w reviewed-1234..pr/1234 to get a diff between what I’ve already reviewed and what is new or changed. git d is git diff with a few options to get diffs as I like them, and white space insensitive diffs are usually the right thing for Go code.
This is all manual and somewhat tedious but it works.
Suggestions?
