On GitHub and Code Reviews

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…


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…)


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.



Maybe Upsource? It’s still in its infancy (despite hitting v2.5 already…), but it’s free for OS, has GitHub integration (insomuch as users can log in with GitHub credentials - not sure if there’s anything else, which might be its downfall here), and I think it ticks your boxes (although it does have pain points of its own).

We use it at work, and although it isn’t perfect, it’s not bad.

Poke me on IRC if you want a more detailed discussion of what it can / can’t do.

1 Like

Interesting, I’ll certainly check it out!

Looks similar to Phabricator, Crucible and so on in that it’s nice but requires hosting the source on it. So no integration with GitHub pull requests…

You can point it at any git repo, so you’d be able to point it at GitHub just fine.

I’m assuming (never tried it) that you can also set up your branch filters to pull in GitHub’s refs/pull/xxx/{head,merge} branches, which means PR’s get pulled in as branches.

Hmm Ok, I will take another look :slight_smile:

I used to do it the same way they do it in kernel mailing lists… Over email, and it was working fine.

Agree that it’s a problem that hasn’t been solved with any modern tools for github.

I think phabricator solves it, but that would require use to move repos.

1 Like

So Upsource handles at least half of my points above (marking things as done, remembering what I have reviewed so I don’t have to do it again), but doesn’t understand rebases. It also requires about a terabyte of RAM to index and view a small repo with a few branches in it…

1 Like

Yeah, it has a few problems (cough Java cough)

Since this topic was revived by a spammer let me take the opportunity to opine that reviewing changes on Github is still shitty, just a bit less shitty than in 2015. And the alternatives are too much work or suck in other ways, including the one the spammer linked to and which I’m not going to mention. So here we are.

This might have been someone’s honest opinion, but due the small mobile screen and a few beers I deleted the account without checking if it’s potentially a genuine user with 10k posts, as it just triggered my spam detector.

Yeah, reviews on github still suck, but having to use yet another tool sucks even more.


No it was definitely spam.