Hacker News new | ask | show | jobs
by depoll 4421 days ago
As far as code reviews go, this is pretty spot on. I was part of a a startup that was using GitHub pull requests for code reviews. As the team grew, it became more and more intractable, although not simply because of notifications. Side-by-side diffs and checkpointed diffs (so that you can see what changed since the last round of review and whether/how your comments were addressed) are handled very poorly by GitHub. We ultimately switched to Phabricator, and while there was a little friction as folks got acquainted with the new tool, it made code reviews a much more pleasant process.

Recently, I had to go through a full code review back on GH pull requests, and it felt like pulling teeth in comparison. They're fine for interacting with contributors to an open source project, but compared to working with a tool like Phabricator that's built for a code reviewer's workflow (and for teams of engineers working together on a project), they just don't hold a candle, in my opinion.

3 comments

Did you also try (or consider) Gerrit Code Review? If so, how does it compare to Phabricator?
I am not a part of the Docker team, but I think Docker handles this huge mess of notifications by gunsub:

https://github.com/jpetazzo/gunsub

tl;dr: automatically unsubscribe from Github issues that you are not directly involved in.

So, you can get a notification for every issue or pull request that opens in the project or organization that you are a member of, as usual, but you don't get bombarded by every notification, e-mail and status update for the duration of the life of that issue, and associated follow-up e-mails. If you want the notifications, just get involved in the conversation.

I am sure Phabricator is very good, and I'll check it out, but I don't have to sell my boss GitHub, he's already sold on it. We just need to get on feature branches and pull requests (at least if we're going to grow.)

As someone who recently implemented Phabricator at our company, it's really an amazing piece of technology. It's easy to install and configure, it does pretty much everything you might expect it to do and then some, and there's a lot of nice-to-haves (like closing tasks via commits, associating commits to tasks, connecting tasks together, etc).

Also, the ability to set up Herald rules to do any notifications you want, like 'add me to CC for any code review that touches this file path in any of these repositories', 'set up an audit whenever someone commits changes to the master branch of this repository', etc. It has a ton of flexibility for integrating with other services, like allowing people to authenticate through their Google account, GitHub, Twitter, JIRA, Disqus, Twitch.tv, Mozilla Persona, and obviously LDAP as well, or normal Phabricator-specific username/password authentication.

It's really insane how much integration there is, how rapidly the development is progressing (we found a bug on a Friday and it was fixed by Monday afternoon), and how responsive the dev team is (in #phabricator on Freenode).

All that for free. Pretty great.

> Side-by-side diffs and checkpointed diffs (so that you can see what changed since the last round of review and whether/how your comments were addressed) are handled very poorly by GitHub.

Did you consider whether that might've come from having pull requests that are simply too big? I've observed that especially neophyte developers tend to make commits and pull requests that are too big and conflate too many issues into one, that are be very handlable once split into smaller logical units.

We did consider that, and put a lot of effort into keeping pull requests small, but the reality is that GH pull requests just don't get the workflow right for a code review that may involve multiple team members, require several passes back and forth, and can handle larger features.

In my experience, pull requests just aren't the right tool for the job at the moment. That's not to say they don't have their place, but there are definitely superior tools out there.