[SciPy-Dev] pull requests and code review

Fernando Perez fperez.net@gmail....
Thu Jan 5 19:58:14 CST 2012


On Thu, Jan 5, 2012 at 5:48 PM, Travis Oliphant <travis@continuum.io> wrote:
> Understood.   I will listen to the feedback --- apologies to those whose
> toes feel steeped on by the crazy cousin stepping back into the house.
>
> The github process is one I am still not used to in practice. Also,
> obviously my orientation is towards much faster review cycles which have to
> happen in industry.

Just commenting from our perspective; IPython being the first project
that switched to github and one that as of late has had a
fast-and-furious rate of PR merging, I think we've found a flow that
works reasonably well, without any hard-and-fast rules:

- We try to review everything via pull requests, except truly trivial
stuff, *small* documentation-only fixes, and emergency, 'my god we
broke everything' fixes that may require a cleanup in a PR later but
that just must go in to ensure master works at all.

- For small PRs, a single person's review may be sufficient.  Anything
reasonably significant, we tend to let it sit after the first review
for at least a day in case someone else has an opinion.  It's also the
case that it's rare that a large PR doesn't require any fixes.  We do
try to read every line of code, and it's rare to see hundreds of lines
of code where every one is perfect out of the gate.

So it very naturally tends to happen that larger PRs have a longer
life, but that's because we really are quite picky: review means check
every line, run the test suite with that branch merged, etc; not just
look a the overall idea and trust the details are OK.

- We make liberal use of the @username feature to directly ping anyone
we suspect may care or may have useful idea/opinion.

- For really large stuff, we tend to wait a little longer, and often
will ping explicitly the whole list saying 'hey, PR # xyz is big and
complicated and has feature foo that's a bit controversial, please
come over and provide some feedback'.


I hope this helps the crazy cousin ease himself back into civilized
society ;)  We're certainly glad to have him!!!

Best,

f


More information about the SciPy-Dev mailing list