Response to “The State of Github’s Code Reviews”

Justin Abrahms has written an article discussing the current state of code reviewing when using Github.

GitHub, the current de facto standard for this

[code language=”reviews”][/code]

, is letting us down.

This is a huge WTF for me. Github isn’t the standard; gerrit, reviewboard, rietveld are. I like how issues with the GitHub interface are pointed out, but this kind of exaggeration….it’s like some programmers are living in a bubble where everyone uses GitHub pull requests. I’ve had difficulty at every place I’ve worked at in first, just getting people to use git, and then getting them to use branches, and now it’s impossible to get them to use pull requests at all. Code reviews, if done at all, are done by inserting text into a file or by face-to-face discussions.

Even popular free/open source projects don’t always do code reviews with tools like Github.

Side by side diffs would be nice, but at the same time, you do have access to git on the command line and graphical interfaces on your own machine. I’ve been able to use magit in Emacs to view side by side diffs, but in the end I always prefer unified diffs.

I find keyboard shortcuts on web sites and web apps to be useless. There’s no need for Github to add them when it will only please a small number of users.

Library detection would be nice but then Github would be in the business of doing checksums on files and there are a lot of files. They’d have to establish a full list of canonical list of library files to match against. It’s nice when longer files and diffs are loaded on demand.

There’s a solution to the library detection thing if you don’t want to see jquery library updates in your diffs. You can set the library files to be recognized as binary files, The Git Book details how to do that. The git attributes file uses the same patterns as the git ignore file so you can match all files within a particular directory or a certain filename and apply the binary attribute. For example, for jQuery you could have “jquery-*.js” or “jquery/” as the pattern in your git attributes file.

I can kinda understand the issue with in-progress reviews but if your pull requests are gigantic, I think having less reviews that are in-depth is better than allowing for viewing the diff of the diff. Again, you can do this locally and you don’t need to rely on GitHub at all for this functionality. You don’t even need to rely on their APIs and wish that they were cleaner APIs; you have the code, you have the information that git provides. That’s all you need for providing the context you want during reviews.

Providing context during the review is a big job and something GitHub would never be able to handle. This is the job of your own build system and your own local tools. If I wanted to, I could have emacs

It feels like a lot of developers are treating GitHub as a central repo which defeats the point of git. You don’t *need* a web interface on some remote machine that could go down at any time.