As far as I can tell, Github's latest additions to their code review flow pretty much brings Github on par with Gerrit (and with also a much larger mind share. Gerrit has always remained quite obscure and marginal).
As far as I can tell, Github's latest additions to their code review flow pretty much brings Github on par with Gerrit (and with also a much larger mind share. Gerrit has always remained quite obscure and marginal).
For example, if a contributor force pushes a new commit to a PR branch, as a reviewer, I can't see the old version and compare against it. I'd have to tell the contributor to not do that (doesn't work for first time contributors), or always try to manually backup each stage of a PR and compare with locally. This doesn't happen on Gerrit because it tracks the entire history of the change.
Second thing. Once a PR gets large and lasts a while, it's hard to see what is going on, what review comments still need to be addressed, and where new replies are showing up. Consider this PR [0] as an example, can you tell where my "latest reply" was? It's not at the bottom because it was a reply to one of the early inline conversations, and you have to expand all the "Show outdated" links manually to find it.
Those are just a few things off the top of my head, as someone who used PRs and Gerrit. I prefer PRs but I don't think they're viable for a project that takes code quality and code review as seriously as Go does. I do hope PRs get the remaining missing pieces to be able to match Gerrit.
> For example, if a contributor force pushes a new commit to a PR
> branch, as a reviewer, I can't see the old version and compare
> against it.
This is not fully true. While you can't "git fetch" that branch
anymore, you can see comments against those old diffs in the pull
request UI, which are the things likely to have changed.For example, I submitted this PR against uWSGI a few months ago: https://github.com/unbit/uwsgi/pull/1392
Initially I screwed it up and inserted a field in the middle of a struct, whereas within a stable release it should of course be added to the end. If you click on "show outdated" you can see that old diff where I'm adding the field to the middle of the struct, but click on "files changed" and it's the current change where I fixed the issue.
I do think GitHub could improve here, they could expose forced pushes via refs/pull/NUM-COUNTER instead of just refs/pull/NUM as they do now, but it's important to point out that they're handling the common case. When someone force-pushes a pull request they're usually doing so in response to comments on the request, and those comments will still make sense because GitHub saves away the old diff, which you can compare with the current state.
Changesets take the typical GitHub workflow where the author force pushes to the PR to address comments and creates another "version" of that PR, with a new set of inline comments, but sharing the overarching PR comments. You can compare and contrast any set of "PR versions" in order to determine the changes applied by the author.
I desperately wish GitHub PRs worked this way.
I regularly collaborate on github with people who are familiar with either Gerrit or Phabricator, and I've yet to see someone praise github for having a better workflow. Much of what they announced recently feels like alpha quality/playing catch up.
I haven't used Gerrit in a few years though, maybe it's changed.
Github has certainly come far this last year, but these are all oh so simple and basic stuff.
The automation would be even more important for a codebase like Chromium that receives 5-8 change lists every minute [2].
We don't need to roll back tree-breaking changes because the bots are the only ones allowed to land changes in the first place, and they make sure all tests pass before they land. There's no reason a bot couldn't do this, though.
That said, Google might consider it easier to work with Gerrit, Rietveld, Gitiles, etc. because they can work with the source code of these projects directly, than having to write potentially ugly code to work around missing pieces in GitHub's API.
I'd love to learn whether a large scale project such as Rust has run into these issues with GitHub.
There's some stuff that could be nicer with GitHub generally, but I don't think having the source would make it any easier to build out this automation; github's API is pretty solid.
I don't think the issues were really that serious, but the Go team likely saw no reason to change away from the tool they were comfortable with for a slightly worse tool that they would have to adapt to.