codereview: accept Github PRs

[bradfitz]

I propose we start accepting Github PRs (Pull Requests).

Currently we have a bot auto-close them with a message telling them we don't use PRs and instead use Gerrit.

When we moved to Github, @robpike said:

Most members of the Go community use Git and host their work on GitHub, and we should join them.

While that's true, we're still not using Github like Github users use Github.

I believe that our current pushback bot dissuades many potential contributors.

I propose we start accepting pull requests by automatically converting them into Gerrit CLs ("change lists", same as a PR but different terminology). Reviews would still happen on Gerrit and the bot would update the PR of activity on Gerrit. Gerrit is still where we'd run trybots and push the "Merge" button. We would never merge on Github. Gerrit would remain the upstream source-of truth.

I prototyped this syncing in https://github.com/LetsUseGerrit/gerritbot/ and used it a bit while working on gRPC-go (examples), but in the opposite direction: my Gerrit CLs were abandoned after gRPC-go accepted them on Github.

In any case, the point is that this can be automated with a bit of work and rejecting Github PRs or not is a policy decision more than anything. I propose we change our policy.

Some will say that the quality of PRs will decrease, as many the PRs that arrive and are auto-closed by the pushback bot are pretty bad. But so are many of the Gerrit CLs. I believe the Gerrit CLs are only better on average because that means it's more likely people have read the contributing instructions, or have contributed in the past. But if you only look at first-time contributors on Github PRs vs Gerrit CLs, the quality doesn't look too different. People improve over time as they learn the project and its policies.

[randall77]

How would the CLA happen? My understanding is that currently you can't make a Gerrit CL without having already jumped through the CLA hoop. If we auto-make a Gerrit CL, we'd have un-CLAd CLs floating around. Mark them in big red letters, maybe?

[bradfitz]

@randall77, Google has an official bot for CLA checking already. We just have to flip a bit to turn it on.

[bradfitz]

(The CLA check would happen on the PR before the Gerrit CL is created.)

[fatih]

Is Gerrit a requirement? I'm asking because Github recently released improved versions of reviewing Code at Github. Some parts (such as commenting on the code) is now similar to Gerrit. Also say we opened a PR from Github, is the Contributor required to do anything in Gerrit or is only used for merging/testing? I'm curious about the boundary (I just contributed once via Gerrit so don't remember quite the details anymore

[mvdan]

What about feedback? If someone files a PR and it gets converted to a Gerrit CL, they might not know how or be willing to reply to any comments or update the code.

[bradfitz]

@fatih, Gerrit is still a requirement. GitHub has made improvements, but it's still not as powerful as Gerrit. Moving off Gerrit would be way too big of a change to be palatable.

@mvdan, as I wrote above:

Reviews would still happen on Gerrit and the bot would update the PR of activity on Gerrit.

You can see this in action at grpc/grpc-go#546 for example. The bot is barely a prototype at this point and could be improved to leave better messages on Github.

[velovix]

Taking a look at the grpc issue, I'm a bit skeptical as to the value this strategy adds. Users get to contribute changes in a more familiar way, but as soon as a code review is posted, we're in unfamiliar territory again. I suspect that, as a result of this change, we'll see a lot of abandoned contributions proposed by people who didn't know what they were getting into and don't have an interest in figuring this all out. Learning Gerrit is a pill contributors still need to swallow, regardless.

That said, I appreciate the effort the Go team is making to improve this process.

[bradfitz]

but as soon as a code review is posted, we're in unfamiliar territory again.

It's something to learn, yes, but it's not as much of a mountain as our current process.

Also, the new Gerrit UI is coming along nicely and will probably be the default at about the same time we're done with this, lowering the bar as well.

[rakyll]

I would also love to see Trybots integration to push the CI results back to the PR. If no one is working on that, I can.

[bradfitz]

@rakyll, that'd be nice, but it's a bit soon for that. Let's get Hello World working first, and then we can do that fanciness.

[emidoots]

( @rakyll did this get closed by accident? )