[Python-Dev] PEP 481 - Migrate Some Supporting Repositories to Git and Github

[Donald Stufft]

> On Nov 29, 2014, at 7:43 PM, Antoine Pitrou <solipsis at pitrou.net> wrote:
> 
> On Sun, 30 Nov 2014 00:15:55 +0000 (UTC)
> Alex Gaynor <alex.gaynor at gmail.com> wrote:
>> 
>> The seconds experience I have is that of Django's migration to git and github.
>> For a long time we were on SVN, and we were very resistant to moving to 
>> DVCS in
>> general, and github in particular. Multiple times I said that I didn't see how
>> exporting a patch and uploading it to trac was more difficult than sending a
>> pull request. That was very wrong on my part.
>> 
>> My primary observation is not about new contributors though, it's actually
>> about the behavior of core developers. Before we were on github, it was fairly
>> rare for core developers to ask for reviews for anything besides *gigantic*
>> patches, we'd mostly just commit stuff to trunk. Since the switch to github,
>> I've seen that core developers are *far* more likely to ask for reviews of
>> their work before merging.
> 
> I don't know anything about Django's old SVN setup, but our code review
> tool (Rietveld) is actually quite good at that, and IMHO slightly
> better than github, since it will only send an e-mail at the final
> submission - by contrast, each individual comment you leave on github
> fires a separate notification, which can feel like spam.
> 
> Our main problem for reviews these days is the lack of core developer
> time. For example, Serhiy often asks for reviews (he has many patches
> pending), which I personally don't have a lot of time to provide.
> 

I think one of the issues with Reitveld isn’t related to Reitveld itself at
all, it’s all the *other* stuff you have to do to get a patch into Reitveld to
allow someone to review it at all. Generating a patch and uploading it to
Roundup is a pain and it’s far easier to just commit things directly to
default.

As far as Reitveld vs Github itself goes for reviews, I don’t personally agree.
Sending a single notification vs a notification per comment is going to be a
very subjective point of view, I like more notifications for the simple fact
that if I'm actively working on a PR while someone is reviewing it I can fix
the issues as quickly as they are finding them.

However if you move past that one thing, the Github PR review has a number of
benefits over Reitveld itself. Since the repositories this PEP currently deals
with are largely documentation-esque repositories the "Prose" diff is
incredibly useful for viewing a content-based diff instead of a diff designed
more for software where you don't normally reflow lines as much. In addition
the Github diff viewer also makes it trivial to expand the context of the diff
so you can see more of the surrounding file, even allowing you to expand it
to the entire file. This is super useful when the automatic amount of context
can't really give you enough information for reviewing the change or not.

Another difference is in how the review comments are presented. With Reitveld
the inline comments go away for each patchset you upload, regardless of if
you've addressed the concerns in that comment or not. They do get bubbled up
into the overall "Messages" view, however this has the opposite problem where
it gives you all of the messages regardless of if they are still a problem
with the latest code or not. In contrast the Github PR will hide old comments
when they lines they addressed have changed, but still make it easy to see the
old comments and the context in which they were made.

There's also the UI itself, it's somewhat dated and I think it's not entirely
unobvious that it suffers from something a lot of OSS projects suffer from in
that it is a "Developer" made UI. I don't think it's a big secret that
developers tend to make UIs that are not as nice as proper UX folks make, but
OSS has historically had a problem attracting that kind of talent (and is
often openly hostile towards it).

Finally there's the transferability of knowledge at play too. I likely would
not review someone else's patch unless I felt strongly about it on Reitveld
largely because I don't know Reitveld very well and I'd rather spend the time
that I'd need to learn to use it effectively working on other projects where
I already know the entire toolchain well. This is one of the larger points in
this PEP that the benefits to using popular tooling is that not only does
people's existing knowledge transfer easily into your project, but the time
they spend learning your tooling also transfers easily outside of this project.
This makes it much more attractive to learn the tooling since the hypothetical
person would be able to take that knowledge and apply it elsewhere.

It is my experience, and this entirely ancedotal, that it's far easier to get
reviews from non-committers and committers alike on projects which are hosted
on Github than it is to get reviews on projects which are not hosted there.

---
Donald Stufft
PGP: 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA