arrow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Wes McKinney <...@cloudera.com>
Subject Re: Code review tools for Arrow patches
Date Fri, 15 Apr 2016 15:48:54 GMT
In my experience, GitHub pull requests are only appropriate for patches
that do not evolve significantly from the first iteration. Changes to
patches frequently cause outstanding points of discussion to be obscured
(the dreaded "comment on an outdated diff").

Rebasing also frequently puts GitHub through a loop.

Too often, as a result, it's tempting to rubber-stamp large patches on
GitHub PRs vs reviewing with great care (which the tool penalizes you for,
IMHO).

Like Jacques my preference is to have a Gerrit instance available that we
can opt in to (I would also like to use Gerrit for parquet-cpp), but not
require a heavier process necessarily for small patches.

On Friday, April 15, 2016, Zheng, Kai <kai.zheng@intel.com> wrote:

> I'm not seeing either is good over the other, but did notice that many
> good discussions in PR reviewing not seen here, though concrete comments
> for some codes in place are very convenient comparing with JIRA based
> reviewing. No one just looks to be perfect.
>
> Regards,
> Kai
>
> -----Original Message-----
> From: Wes McKinney [mailto:wes@cloudera.com <javascript:;>]
> Sent: Friday, April 15, 2016 1:38 AM
> To: dev@arrow.apache.org <javascript:;>
> Subject: Code review tools for Arrow patches
>
> hello all,
>
> We're reaching a junction where larger patches to the Arrow codebase will
> become more frequent, and effective code reviews will be important part of
> maintaining a high quality project going forward.
>
> In general, the GitHub pull request UI is not generally thought of as very
> productive in large code reviews (some recent exposition on this
> topic:
> http://www.beepsend.com/2016/04/05/abandoning-gitflow-github-favour-gerrit/
> ).
> Many large engineering teams prefer such (git-centric) tools as Gerrit,
> though there are other code review tools available.
>
> I don't think we are at a point where a particular code review process
> should be enforced, but more that we should have more tools available for
> groups of Arrow committers who wish to collaborate in a particular way.
>
> As I'm familiar with Gerrit from working on Apache projects that
> Cloudera's involved with, my bias would be to try to get an instance set up
> so that larger patches can be reviewed in a more detailed and transactional
> way. For example: we could use gerrit.cloudera.org (like Kudu and
> Impala), but I would be happy to use any infrastructure provider.
>
> There has been some resistance / inaction within the ASF to create an
> ASF-managed Gerrit instance:
> https://issues.apache.org/jira/browse/INFRA-2205.
>
> I'm interested to hear other perspectives.
>
> Thanks,
> Wes
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message