
Go Proposal: Accept GitHub PRs - zalmoxes
https://github.com/golang/go/issues/18517
======
hota_mazi
I'd be curious to hear more about why the Go team thinks Gerrit is superior to
Github for code reviews.

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

~~~
jzelinskie
Changesets are a big deal for seeing progress towards code review feedback.

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.

~~~
eridius
I use a tool [https://reviewable.io/](https://reviewable.io/) which adds this
functionality (and some other stuff) on top of GitHub PRs and it's invaluable.

~~~
helper
+1 for reviewable. In addition to this feature reviewable's diff display is a
lot nicer. For example it does a much better job handling whitespace only
changes when a block of code gets reindented but otherwise doesn't change.

~~~
eridius
Oh yeah, collapsing whitespace-only changes is excellent. But even better is
how it can detect and collapse changes introduced by rebasing the PR.

~~~
petetnt
You can hide whitespace-only changes in GitHub by appending ?w=1 to the end of
the url too.

~~~
eridius
But that makes the whitespace changes literally invisible. Reviewable strikes
a good balance, where it will show you whitespace changes that appear in diff
hunk contexts, and also explicitly call out when it's skipping a chunk of
whitespace-only changes, and you can click on that callout to ask it to show
you those whitespace changes anyway.

------
Manishearth
With Servo we use [https://reviewable.io/](https://reviewable.io/) on github.

We don't enforce its use. If a newcomer makes a pull request, we'll just use
the GH review interface. In general simple PRs go through the GH review
interface, regardless of who they're from. We break out Reviewable when a PR
actually _needs_ it (this is usually the choice of the reviewer).

This means that Servo continues to have a lower bar to contribution, without
hampering maintainers on PRs that need more than GHs set of review features.

This has worked out well for us.

~~~
StevePerkins
I've contributed to another Google project
([http://vitess.io](http://vitess.io)) that used the the same process. So
while Go is a completely different team, it's not exactly unknown within
Google. Reviewable is a dramatic improvement over the stock Github tooling.

------
sergiotapia
Seems like a lot of hoops to jump through, I wonder why they want to use
Gerrit. Meanwhile Elixir you open a PR and Jose and the gang address it
quickly.

4 open PR's! [https://github.com/elixir-
lang/elixir/pulls](https://github.com/elixir-lang/elixir/pulls)

~~~
strictfp
Hey, we're getting downvoted! I guess gophers aren't so pragmatic after all, I
thought they followed New Jersey style!

------
strictfp
I can't help thinking that requiring a specific code review tool is a bit
ridiculous, especially considering the hoops they want to jump through to
enforce it.

~~~
dom0
Because maintainer timer is, in your world, a worthless thing?

~~~
strictfp
No, because the tools are very similar and they can just ask contributors to
adapt to the maintaners wishes. Code diffs are such a small part of reviewing
a new feature. "Just deal with it" is what I think. I don't require notepad on
my Mac, so why should they require gerrit on github? And besides, if they want
to they can just diff it on their own machines with whatever tools they like!

~~~
dom0
Gerrit and Github are not "very similar". They are quite different. (Something
that would be rather immediately clear to anyone who used either for some
duration)

> And besides, if they want to they can just diff it on their own machines
> with whatever tools they like!

Oh yes, a very compelling argument when discussing _workflow tools_ is to
suggest local diffs. Bravo!

~~~
strictfp
Well, I happen to have used both. And it all depends on your perspective. I
would change my tools in a heartbeat to streamline the process for users or
contributors. To me it depends on if you're user focused or technology driven.
I think it's kind of akin to switching to git but insist that all merges have
been done with perforce merge, because I think it's better in some way. Sure,
it might be true, but do you really want to go against the stream to prove
your point?

------
yetanother1
I contribute pull requests to quite a few github projects. However I avoid
contributing to (or even using) projects with a Contributor License Agreement.
Such projects are too bureaucratic.

~~~
pcl
> I avoid contributing to (or even _using_ ) projects with a Contributor
> License Agreement.

That's a pretty tough thing to do. Most (all?) Apache projects require a CLA,
as does Go, Java, Mysql, Cassandra... It's pretty standard for any project
that is both managed by a corporation and accepts third-party contributions.

EDIT: removed Postgres. Thanks, 'anarazel!

~~~
yetanother1
~~You got me on Postgres. Don't use the others.~~

Edit: another post states that Postgres doesn't require a CLA for
contributors. So I don't use any software requiring a CLA.

