[Pandas-dev] GitHub's new code-review tools

Wes McKinney wesmckinn at gmail.com
Wed Sep 14 23:24:21 EDT 2016


This helps by reducing some of the e-mail chatter / grouping together
sets of comments.

Biggest outstanding weakness I see versus other tools (e.g. Gerrit) is
that it does not seem to support reviews of incremental changes to the
PR. More mature review tools allow you to see how each comment was
addressed in the incremental patch. On larger / more complex patches,
reviews may go through several rounds of incremental changes. Multiple
times in the last year I've had to make 3 or more careful passes
through patches over 1000 lines. You can leave comments on individual
commits, but that presumes that each commit is meaningful (this is
distinct from the notion of patch sets -- transactional changes to the
patch -- used in Rietveld, Gerrit, etc.)

To give you a concrete example, I participated in this code review
earlier this year which went through 17 patch sets:

https://gerrit.cloudera.org/#/c/2645/

Here's an example of an incremental diff; this shows comments between
patch sets 15 and 16 and how the comments were addressed:

https://gerrit.cloudera.org/#/c/2645/15..16/src/hs2client/operation.cc

I continue to worry that GitHub makes it too tempting to just rubber
stamp larger PRs or leave less detailed / more superficial commentary.

(for historical commentary on the origin of these review tools:
https://en.wikipedia.org/wiki/Rietveld_(software) )

- Wes

On Wed, Sep 14, 2016 at 2:47 PM, Christopher Aycock
<Christopher.Aycock at twosigma.com> wrote:
> Hot off the presses, GitHub has more robust code-review tools:
>
>
>
> https://github.com/blog/2256-a-whole-new-github-universe-announcing-new-tools-forums-and-features
>
>
>
> Do these features help at all with the current problems reviewers have been
> having?
>
>
>
>
> _______________________________________________
> Pandas-dev mailing list
> Pandas-dev at python.org
> https://mail.python.org/mailman/listinfo/pandas-dev
>


More information about the Pandas-dev mailing list