You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Wes McKinney <we...@cloudera.com> on 2016/04/14 19:38:21 UTC
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
Re: Code review tools for Arrow patches
Posted by Jacques Nadeau <ja...@apache.org>.
My preference in order is:
1. Gerrit
2. Review board
3. GH PRs
The lack of transactionality and good history makes GH PRs frequently
inadequate. (We used them as the default on Drill for the last year.) The
per comment email model is also ridiculous.
On Thu, Apr 14, 2016 at 3:27 PM, Henry Saputra <he...@gmail.com>
wrote:
> How about using Github Pull Requests as mechanism to do review?
>
> So, all patches need to be done via Github mirror PRs, similar to Flink and
> Spark and other new projects have been doing.
>
> At AsterixDB we did Gerrit and has to do some tooling to make sure it
> comply with ASF way.
>
> - Henry
>
> On Thu, Apr 14, 2016 at 11:07 AM, Wang, Yanping <ya...@intel.com>
> wrote:
>
> > I agree, we need a code review tools as well for Mnemonic. We are going
> to
> > submit large patches. I include mnemonic dev list here for discussion.
> >
> > Thanks
> >
> > > On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
> > >
> > > 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
> >
>
Re: Code review tools for Arrow patches
Posted by Jacques Nadeau <ja...@apache.org>.
My preference in order is:
1. Gerrit
2. Review board
3. GH PRs
The lack of transactionality and good history makes GH PRs frequently
inadequate. (We used them as the default on Drill for the last year.) The
per comment email model is also ridiculous.
On Thu, Apr 14, 2016 at 3:27 PM, Henry Saputra <he...@gmail.com>
wrote:
> How about using Github Pull Requests as mechanism to do review?
>
> So, all patches need to be done via Github mirror PRs, similar to Flink and
> Spark and other new projects have been doing.
>
> At AsterixDB we did Gerrit and has to do some tooling to make sure it
> comply with ASF way.
>
> - Henry
>
> On Thu, Apr 14, 2016 at 11:07 AM, Wang, Yanping <ya...@intel.com>
> wrote:
>
> > I agree, we need a code review tools as well for Mnemonic. We are going
> to
> > submit large patches. I include mnemonic dev list here for discussion.
> >
> > Thanks
> >
> > > On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
> > >
> > > 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
> >
>
Re: Code review tools for Arrow patches
Posted by Henry Saputra <he...@gmail.com>.
How about using Github Pull Requests as mechanism to do review?
So, all patches need to be done via Github mirror PRs, similar to Flink and
Spark and other new projects have been doing.
At AsterixDB we did Gerrit and has to do some tooling to make sure it
comply with ASF way.
- Henry
On Thu, Apr 14, 2016 at 11:07 AM, Wang, Yanping <ya...@intel.com>
wrote:
> I agree, we need a code review tools as well for Mnemonic. We are going to
> submit large patches. I include mnemonic dev list here for discussion.
>
> Thanks
>
> > On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
> >
> > 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
>
Re: Code review tools for Arrow patches
Posted by Henry Saputra <he...@gmail.com>.
How about using Github Pull Requests as mechanism to do review?
So, all patches need to be done via Github mirror PRs, similar to Flink and
Spark and other new projects have been doing.
At AsterixDB we did Gerrit and has to do some tooling to make sure it
comply with ASF way.
- Henry
On Thu, Apr 14, 2016 at 11:07 AM, Wang, Yanping <ya...@intel.com>
wrote:
> I agree, we need a code review tools as well for Mnemonic. We are going to
> submit large patches. I include mnemonic dev list here for discussion.
>
> Thanks
>
> > On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
> >
> > 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
>
Re: Code review tools for Arrow patches
Posted by "Wang, Yanping" <ya...@intel.com>.
I agree, we need a code review tools as well for Mnemonic. We are going to submit large patches. I include mnemonic dev list here for discussion.
Thanks
> On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
>
> 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
Re: Code review tools for Arrow patches
Posted by Stack <st...@duboce.net>.
On Tue, May 3, 2016 at 6:59 AM, Jacques Nadeau <ja...@apache.org> wrote:
> I dont know about the other pmc members and committers but I prefer just
> making Gerrit the only way to submit patches rather than one of many. It
> seems to work well for Asterix and Kudu.
>
I'd say go for it. Can always undo if it proves too high a bar for new
contributors to get over.
St.Ack
> On Apr 25, 2016 12:16 PM, "Todd Lipcon" <to...@cloudera.com> wrote:
>
> > On Mon, Apr 25, 2016 at 12:09 PM, Wes McKinney <we...@cloudera.com> wrote:
> >
> > > hi Todd,
> > >
> > > This is helpful to know, thank you. As you say, by "optional" what I
> > > meant was that code could be optionally reviewed in Gerrit, but then
> > > commits would have to be cherry-picked by the committer from the
> > > Gerrit git remote. We would continue to accept patches via GitHub pull
> > > requests (but for larger patches, they may move from GitHub to gerrit
> > > as need be). Unless there is some pitfall here that I'm missing.
> > >
> >
> > The pitfall is what the state of the gerrit remote looks like. How do you
> > keep it up to date with the ASF repo, if you have patches entering the
> ASF
> > repo from some mechanism other than gerrit?
> >
> > It might be possible involving some cron job which force pushes from ASF
> ->
> > Gerrit, but I haven't ever tried a workflow like that.
> >
> > -Todd
> >
> >
> > >
> > > On Mon, Apr 25, 2016 at 3:03 PM, Todd Lipcon <to...@cloudera.com>
> wrote:
> > > > Using gerrit as an "optional" tool is a bit difficult, because it
> > doesn't
> > > > know how to handle commits to a repository that it doesn't own.
> > > >
> > > > The way we get around the "commit via gerrit" issue in the Kudu
> podling
> > > is
> > > > to follow the example of AsterixDB. Commits are made using gerrit,
> but
> > > that
> > > > doesn't automatically flow to the ASF repo. The committer then runs a
> > > > 'push-to-asf.py' script which grabs the commit from gerrit and pushes
> > to
> > > > the ASF repository:
> > > >
> > >
> >
> https://github.com/apache/incubator-kudu/blob/master/build-support/push_to_asf.py
> > > >
> > > > I'm happy to set up the gerrit projects, but not sure how it would
> work
> > > in
> > > > an "optional" context.
> > > >
> > > > -Todd
> > > >
> > > >
> > > > On Sun, Apr 24, 2016 at 4:53 PM, Julian Hyde <jhyde.apache@gmail.com
> >
> > > wrote:
> > > >
> > > >> IIRC Apex wanted to commit via Gerrit. That was a non-starter.
> Commits
> > > >> have to be made by a committer.
> > > >>
> > > >> Julian
> > > >>
> > > >>
> > > >> > On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com>
> wrote:
> > > >> >
> > > >> > Sending all Gerrit review activity to the mailing list seems
> > adequate
> > > to
> > > >> me.
> > > >> > I don't see how this is especially different from reviewing code
> on
> > a
> > > >> > website owned by GitHub. I remain hopeful that ASF Infra will set
> up
> > > an
> > > >> > ASF-managed Gerrit.
> > > >> >
> > > >> > On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com>
> > wrote:
> > > >> >
> > > >> >> Just for the record, Apex had some issues getting Gerrit reviews
> > > >> reflected
> > > >> >> in a coherent fashion into the Apache record. I presume that you
> > guys
> > > >> will
> > > >> >> have that handled or will check with the Apex devs to learn their
> > > >> >> resolution.
> > > >> >>
> > > >> >>
> > > >> >>
> > > >>
> > > >>
> > > >
> > > >
> > > > --
> > > > Todd Lipcon
> > > > Software Engineer, Cloudera
> > >
> >
> >
> >
> > --
> > Todd Lipcon
> > Software Engineer, Cloudera
> >
>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@gmail.com>.
For me, at least, it's a time constraint. It should be possible to
replicate our current setup on Jenkins -- it would be useful to
continue to run Travis CI, but perhaps the builds can share common
shell scripts.
On Sat, Jun 11, 2016 at 12:56 PM, Jacques Nadeau <ja...@apache.org> wrote:
> Is the challenge with jenkins + gerrit a time constraint or a systems
> constraint? If the former, we can probably have other people collaborate to
> accomplish this. (If I understand what you said.)
>
>
> On Fri, Jun 10, 2016 at 3:41 PM, Wes McKinney <we...@gmail.com> wrote:
>
>> I am not sure how to do this while continuing to use Travis CI. I am
>> not able to set up a new CI environment (e.g. Jenkins + gerrit a la
>> Kudu) right now.
>>
>> I am having a hard time keeping track of the state of code reviews, so
>> I've proposed this triage solution (which will involve an extra force
>> push to get a green build) to assist with large reviews until we
>> achieve a more sustainable / streamlined solution (Jenkins + gerrit
>> replication, maybe someday).
>>
>> - Wes
>>
>> On Thu, Jun 9, 2016 at 7:53 PM, Jacques Nadeau <ja...@apache.org> wrote:
>> > I'm +1 if we remove step 4 and integrate testing into gerrit instead
>> >
>> > On Thu, Jun 9, 2016 at 1:45 PM, Wes McKinney <we...@gmail.com>
>> wrote:
>> >
>> >> hi folks,
>> >>
>> >> Following up on this. My suggestion for a workflow to help with large
>> >> code reviews for Arrow is:
>> >>
>> >> 1) We set up an Arrow project on gerrit.cloudera.org. I'm hoping we
>> >> see gerrit.apache.org someday.
>> >>
>> >> 2) For reviews needing more careful scrutiny, code reviewer can
>> >> request to conduct the CR on Gerrit
>> >>
>> >> 3) Contributor will push change sets to Gerrit
>> >>
>> >> 4) [The slightly awkward part] In parallel, contributor will open a PR
>> >> on GitHub for purposes of trigging Travis CI verification
>> >>
>> >> 5) Arrow committer may cherry-pick verified commits to master and push
>> >> to ASF git repo
>> >>
>> >> My understanding (someone more experienced should chime in) is that
>> >> Gerrit reviews are all made relative to the parent commit for a
>> >> particular change set. Thus, we may not need to worry (for now) about
>> >> synchronization issues between Gerrit and GitHub / ASF git.
>> >>
>> >> Does this make sense? Any other ideas / thoughts welcome
>> >>
>> >> Thanks,
>> >> Wes
>> >>
>> >> On Mon, May 23, 2016 at 3:55 PM, Hanifi GUNES <ha...@gmail.com>
>> >> wrote:
>> >> > I worked with Gerrit and GH both. My personal preference would be in
>> >> favor
>> >> > of Gerrit because of its power user ready-ness and tight integration
>> with
>> >> > git + git cli. Afaics there are legitimate concerns around possible
>> >> > trickiness of novice users' interaction with Gerrit. Not sure if this
>> was
>> >> > mentioned above but there seems a Gerrit + GH plugin that mirrors GH
>> >> > pull-requests to Gerrit changes. Never used it but still this may be
>> of
>> >> > help.
>> >> >
>> >> >
>> >> > 1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
>> >> >
>> >> > 2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
>> >> >
>> >> >> If everyone else would prefer Gerrit, I would be okay with using it
>> >> >> exclusively to simplify things. It does have several nice features
>> >> beyond
>> >> >> reviewboard as it manages its own git repository, rather than just
>> patch
>> >> >> files.
>> >> >>
>> >> >> Jason Altekruse
>> >> >> Software Engineer at Dremio
>> >> >> Apache Drill Committer
>> >> >>
>> >> >> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
>> >> >> wrote:
>> >> >>
>> >> >> > Apparently it is possible, but quite a lot of work:
>> >> >> >
>> >> >> > https://github.com/andygrunwald/gotrap
>> >> >> >
>> >> >> > The ideal thing, it would seem, would be to have the Gerrit code
>> >> >> > reviews with automatic replication of updated patch sets to a pull
>> >> >> > request (i.e. each new patch set force pushes the branch). I don't
>> >> >> > think we're going to get that, so I'm not sure how to proceed. The
>> >> >> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
>> >> >> > http://gerrit.cloudera.org:8080/#/c/2992/)
>> >> >> >
>> >> >> > - Wes
>> >> >> >
>> >> >> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <
>> >> emkornfield@gmail.com>
>> >> >> > wrote:
>> >> >> > > Does gerrit work well with TravisCI, or will we need to
>> >> develop/setup
>> >> >> > > another continuous integration solution?
>> >> >> > >
>> >> >> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
>> >> >> > > <da...@gmail.com> wrote:
>> >> >> > >> Admittedly, coming from the complete opposite end of the
>> >> commit-size
>> >> >> > spectrum, the JIRA issue + GitHub pull request workflow already
>> feels
>> >> a
>> >> >> > little frictional for simple bugfixes and additions, so I was wary
>> of
>> >> >> > Gerrit. But it actually looks pretty well-suited to small commits.
>> >> >> > >> One advantage I'd see to different platforms, though, would be
>> the
>> >> >> > potential for JIRA integration. GitHub seems to have a more
>> built-in
>> >> >> > solution for this, if it's something you could foresee setting up.
>> But
>> >> >> > there seem to be ways to do it with Gerrit too.
>> >> >> > >> Clearly having an option to use GitHub pull requests lowers the
>> >> >> > barriers to entry for contributors, but I understand easy pull
>> >> requests
>> >> >> are
>> >> >> > a double-edged sword for maintainers!
>> >> >> > >>
>> >> >> > >>
>> >> >> > >>
>> >> >> > >> _____________________________
>> >> >> > >> From: Wes McKinney <we...@gmail.com>
>> >> >> > >> Sent: Thursday, May 5, 2016 12:46 AM
>> >> >> > >> Subject: Re: Code review tools for Arrow patches
>> >> >> > >> To: <de...@arrow.apache.org>
>> >> >> > >>
>> >> >> > >>
>> >> >> > >> I'm also on board with this if it doesn't deter new contributors
>> >> (it's
>> >> >> > >> a bit of additional process over GitHub but overall not too
>> hard to
>> >> >> > >> learn).
>> >> >> > >>
>> >> >> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <
>> jacques@apache.org
>> >> >
>> >> >> > wrote:
>> >> >> > >>> I dont know about the other pmc members and committers but I
>> >> prefer
>> >> >> > just
>> >> >> > >>> making Gerrit the only way to submit patches rather than one of
>> >> many.
>> >> >> > It
>> >> >> > >>> seems to work well for Asterix and Kudu.
>> >> >> > >>
>> >> >> > >>
>> >> >> > >>
>> >> >> > >>
>> >> >> >
>> >> >>
>> >>
>>
Re: Code review tools for Arrow patches
Posted by Jacques Nadeau <ja...@apache.org>.
Is the challenge with jenkins + gerrit a time constraint or a systems
constraint? If the former, we can probably have other people collaborate to
accomplish this. (If I understand what you said.)
On Fri, Jun 10, 2016 at 3:41 PM, Wes McKinney <we...@gmail.com> wrote:
> I am not sure how to do this while continuing to use Travis CI. I am
> not able to set up a new CI environment (e.g. Jenkins + gerrit a la
> Kudu) right now.
>
> I am having a hard time keeping track of the state of code reviews, so
> I've proposed this triage solution (which will involve an extra force
> push to get a green build) to assist with large reviews until we
> achieve a more sustainable / streamlined solution (Jenkins + gerrit
> replication, maybe someday).
>
> - Wes
>
> On Thu, Jun 9, 2016 at 7:53 PM, Jacques Nadeau <ja...@apache.org> wrote:
> > I'm +1 if we remove step 4 and integrate testing into gerrit instead
> >
> > On Thu, Jun 9, 2016 at 1:45 PM, Wes McKinney <we...@gmail.com>
> wrote:
> >
> >> hi folks,
> >>
> >> Following up on this. My suggestion for a workflow to help with large
> >> code reviews for Arrow is:
> >>
> >> 1) We set up an Arrow project on gerrit.cloudera.org. I'm hoping we
> >> see gerrit.apache.org someday.
> >>
> >> 2) For reviews needing more careful scrutiny, code reviewer can
> >> request to conduct the CR on Gerrit
> >>
> >> 3) Contributor will push change sets to Gerrit
> >>
> >> 4) [The slightly awkward part] In parallel, contributor will open a PR
> >> on GitHub for purposes of trigging Travis CI verification
> >>
> >> 5) Arrow committer may cherry-pick verified commits to master and push
> >> to ASF git repo
> >>
> >> My understanding (someone more experienced should chime in) is that
> >> Gerrit reviews are all made relative to the parent commit for a
> >> particular change set. Thus, we may not need to worry (for now) about
> >> synchronization issues between Gerrit and GitHub / ASF git.
> >>
> >> Does this make sense? Any other ideas / thoughts welcome
> >>
> >> Thanks,
> >> Wes
> >>
> >> On Mon, May 23, 2016 at 3:55 PM, Hanifi GUNES <ha...@gmail.com>
> >> wrote:
> >> > I worked with Gerrit and GH both. My personal preference would be in
> >> favor
> >> > of Gerrit because of its power user ready-ness and tight integration
> with
> >> > git + git cli. Afaics there are legitimate concerns around possible
> >> > trickiness of novice users' interaction with Gerrit. Not sure if this
> was
> >> > mentioned above but there seems a Gerrit + GH plugin that mirrors GH
> >> > pull-requests to Gerrit changes. Never used it but still this may be
> of
> >> > help.
> >> >
> >> >
> >> > 1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
> >> >
> >> > 2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
> >> >
> >> >> If everyone else would prefer Gerrit, I would be okay with using it
> >> >> exclusively to simplify things. It does have several nice features
> >> beyond
> >> >> reviewboard as it manages its own git repository, rather than just
> patch
> >> >> files.
> >> >>
> >> >> Jason Altekruse
> >> >> Software Engineer at Dremio
> >> >> Apache Drill Committer
> >> >>
> >> >> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
> >> >> wrote:
> >> >>
> >> >> > Apparently it is possible, but quite a lot of work:
> >> >> >
> >> >> > https://github.com/andygrunwald/gotrap
> >> >> >
> >> >> > The ideal thing, it would seem, would be to have the Gerrit code
> >> >> > reviews with automatic replication of updated patch sets to a pull
> >> >> > request (i.e. each new patch set force pushes the branch). I don't
> >> >> > think we're going to get that, so I'm not sure how to proceed. The
> >> >> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
> >> >> > http://gerrit.cloudera.org:8080/#/c/2992/)
> >> >> >
> >> >> > - Wes
> >> >> >
> >> >> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <
> >> emkornfield@gmail.com>
> >> >> > wrote:
> >> >> > > Does gerrit work well with TravisCI, or will we need to
> >> develop/setup
> >> >> > > another continuous integration solution?
> >> >> > >
> >> >> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
> >> >> > > <da...@gmail.com> wrote:
> >> >> > >> Admittedly, coming from the complete opposite end of the
> >> commit-size
> >> >> > spectrum, the JIRA issue + GitHub pull request workflow already
> feels
> >> a
> >> >> > little frictional for simple bugfixes and additions, so I was wary
> of
> >> >> > Gerrit. But it actually looks pretty well-suited to small commits.
> >> >> > >> One advantage I'd see to different platforms, though, would be
> the
> >> >> > potential for JIRA integration. GitHub seems to have a more
> built-in
> >> >> > solution for this, if it's something you could foresee setting up.
> But
> >> >> > there seem to be ways to do it with Gerrit too.
> >> >> > >> Clearly having an option to use GitHub pull requests lowers the
> >> >> > barriers to entry for contributors, but I understand easy pull
> >> requests
> >> >> are
> >> >> > a double-edged sword for maintainers!
> >> >> > >>
> >> >> > >>
> >> >> > >>
> >> >> > >> _____________________________
> >> >> > >> From: Wes McKinney <we...@gmail.com>
> >> >> > >> Sent: Thursday, May 5, 2016 12:46 AM
> >> >> > >> Subject: Re: Code review tools for Arrow patches
> >> >> > >> To: <de...@arrow.apache.org>
> >> >> > >>
> >> >> > >>
> >> >> > >> I'm also on board with this if it doesn't deter new contributors
> >> (it's
> >> >> > >> a bit of additional process over GitHub but overall not too
> hard to
> >> >> > >> learn).
> >> >> > >>
> >> >> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <
> jacques@apache.org
> >> >
> >> >> > wrote:
> >> >> > >>> I dont know about the other pmc members and committers but I
> >> prefer
> >> >> > just
> >> >> > >>> making Gerrit the only way to submit patches rather than one of
> >> many.
> >> >> > It
> >> >> > >>> seems to work well for Asterix and Kudu.
> >> >> > >>
> >> >> > >>
> >> >> > >>
> >> >> > >>
> >> >> >
> >> >>
> >>
>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@gmail.com>.
I am not sure how to do this while continuing to use Travis CI. I am
not able to set up a new CI environment (e.g. Jenkins + gerrit a la
Kudu) right now.
I am having a hard time keeping track of the state of code reviews, so
I've proposed this triage solution (which will involve an extra force
push to get a green build) to assist with large reviews until we
achieve a more sustainable / streamlined solution (Jenkins + gerrit
replication, maybe someday).
- Wes
On Thu, Jun 9, 2016 at 7:53 PM, Jacques Nadeau <ja...@apache.org> wrote:
> I'm +1 if we remove step 4 and integrate testing into gerrit instead
>
> On Thu, Jun 9, 2016 at 1:45 PM, Wes McKinney <we...@gmail.com> wrote:
>
>> hi folks,
>>
>> Following up on this. My suggestion for a workflow to help with large
>> code reviews for Arrow is:
>>
>> 1) We set up an Arrow project on gerrit.cloudera.org. I'm hoping we
>> see gerrit.apache.org someday.
>>
>> 2) For reviews needing more careful scrutiny, code reviewer can
>> request to conduct the CR on Gerrit
>>
>> 3) Contributor will push change sets to Gerrit
>>
>> 4) [The slightly awkward part] In parallel, contributor will open a PR
>> on GitHub for purposes of trigging Travis CI verification
>>
>> 5) Arrow committer may cherry-pick verified commits to master and push
>> to ASF git repo
>>
>> My understanding (someone more experienced should chime in) is that
>> Gerrit reviews are all made relative to the parent commit for a
>> particular change set. Thus, we may not need to worry (for now) about
>> synchronization issues between Gerrit and GitHub / ASF git.
>>
>> Does this make sense? Any other ideas / thoughts welcome
>>
>> Thanks,
>> Wes
>>
>> On Mon, May 23, 2016 at 3:55 PM, Hanifi GUNES <ha...@gmail.com>
>> wrote:
>> > I worked with Gerrit and GH both. My personal preference would be in
>> favor
>> > of Gerrit because of its power user ready-ness and tight integration with
>> > git + git cli. Afaics there are legitimate concerns around possible
>> > trickiness of novice users' interaction with Gerrit. Not sure if this was
>> > mentioned above but there seems a Gerrit + GH plugin that mirrors GH
>> > pull-requests to Gerrit changes. Never used it but still this may be of
>> > help.
>> >
>> >
>> > 1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
>> >
>> > 2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
>> >
>> >> If everyone else would prefer Gerrit, I would be okay with using it
>> >> exclusively to simplify things. It does have several nice features
>> beyond
>> >> reviewboard as it manages its own git repository, rather than just patch
>> >> files.
>> >>
>> >> Jason Altekruse
>> >> Software Engineer at Dremio
>> >> Apache Drill Committer
>> >>
>> >> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
>> >> wrote:
>> >>
>> >> > Apparently it is possible, but quite a lot of work:
>> >> >
>> >> > https://github.com/andygrunwald/gotrap
>> >> >
>> >> > The ideal thing, it would seem, would be to have the Gerrit code
>> >> > reviews with automatic replication of updated patch sets to a pull
>> >> > request (i.e. each new patch set force pushes the branch). I don't
>> >> > think we're going to get that, so I'm not sure how to proceed. The
>> >> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
>> >> > http://gerrit.cloudera.org:8080/#/c/2992/)
>> >> >
>> >> > - Wes
>> >> >
>> >> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <
>> emkornfield@gmail.com>
>> >> > wrote:
>> >> > > Does gerrit work well with TravisCI, or will we need to
>> develop/setup
>> >> > > another continuous integration solution?
>> >> > >
>> >> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
>> >> > > <da...@gmail.com> wrote:
>> >> > >> Admittedly, coming from the complete opposite end of the
>> commit-size
>> >> > spectrum, the JIRA issue + GitHub pull request workflow already feels
>> a
>> >> > little frictional for simple bugfixes and additions, so I was wary of
>> >> > Gerrit. But it actually looks pretty well-suited to small commits.
>> >> > >> One advantage I'd see to different platforms, though, would be the
>> >> > potential for JIRA integration. GitHub seems to have a more built-in
>> >> > solution for this, if it's something you could foresee setting up. But
>> >> > there seem to be ways to do it with Gerrit too.
>> >> > >> Clearly having an option to use GitHub pull requests lowers the
>> >> > barriers to entry for contributors, but I understand easy pull
>> requests
>> >> are
>> >> > a double-edged sword for maintainers!
>> >> > >>
>> >> > >>
>> >> > >>
>> >> > >> _____________________________
>> >> > >> From: Wes McKinney <we...@gmail.com>
>> >> > >> Sent: Thursday, May 5, 2016 12:46 AM
>> >> > >> Subject: Re: Code review tools for Arrow patches
>> >> > >> To: <de...@arrow.apache.org>
>> >> > >>
>> >> > >>
>> >> > >> I'm also on board with this if it doesn't deter new contributors
>> (it's
>> >> > >> a bit of additional process over GitHub but overall not too hard to
>> >> > >> learn).
>> >> > >>
>> >> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <jacques@apache.org
>> >
>> >> > wrote:
>> >> > >>> I dont know about the other pmc members and committers but I
>> prefer
>> >> > just
>> >> > >>> making Gerrit the only way to submit patches rather than one of
>> many.
>> >> > It
>> >> > >>> seems to work well for Asterix and Kudu.
>> >> > >>
>> >> > >>
>> >> > >>
>> >> > >>
>> >> >
>> >>
>>
Re: Code review tools for Arrow patches
Posted by Jacques Nadeau <ja...@apache.org>.
I'm +1 if we remove step 4 and integrate testing into gerrit instead
On Thu, Jun 9, 2016 at 1:45 PM, Wes McKinney <we...@gmail.com> wrote:
> hi folks,
>
> Following up on this. My suggestion for a workflow to help with large
> code reviews for Arrow is:
>
> 1) We set up an Arrow project on gerrit.cloudera.org. I'm hoping we
> see gerrit.apache.org someday.
>
> 2) For reviews needing more careful scrutiny, code reviewer can
> request to conduct the CR on Gerrit
>
> 3) Contributor will push change sets to Gerrit
>
> 4) [The slightly awkward part] In parallel, contributor will open a PR
> on GitHub for purposes of trigging Travis CI verification
>
> 5) Arrow committer may cherry-pick verified commits to master and push
> to ASF git repo
>
> My understanding (someone more experienced should chime in) is that
> Gerrit reviews are all made relative to the parent commit for a
> particular change set. Thus, we may not need to worry (for now) about
> synchronization issues between Gerrit and GitHub / ASF git.
>
> Does this make sense? Any other ideas / thoughts welcome
>
> Thanks,
> Wes
>
> On Mon, May 23, 2016 at 3:55 PM, Hanifi GUNES <ha...@gmail.com>
> wrote:
> > I worked with Gerrit and GH both. My personal preference would be in
> favor
> > of Gerrit because of its power user ready-ness and tight integration with
> > git + git cli. Afaics there are legitimate concerns around possible
> > trickiness of novice users' interaction with Gerrit. Not sure if this was
> > mentioned above but there seems a Gerrit + GH plugin that mirrors GH
> > pull-requests to Gerrit changes. Never used it but still this may be of
> > help.
> >
> >
> > 1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
> >
> > 2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
> >
> >> If everyone else would prefer Gerrit, I would be okay with using it
> >> exclusively to simplify things. It does have several nice features
> beyond
> >> reviewboard as it manages its own git repository, rather than just patch
> >> files.
> >>
> >> Jason Altekruse
> >> Software Engineer at Dremio
> >> Apache Drill Committer
> >>
> >> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
> >> wrote:
> >>
> >> > Apparently it is possible, but quite a lot of work:
> >> >
> >> > https://github.com/andygrunwald/gotrap
> >> >
> >> > The ideal thing, it would seem, would be to have the Gerrit code
> >> > reviews with automatic replication of updated patch sets to a pull
> >> > request (i.e. each new patch set force pushes the branch). I don't
> >> > think we're going to get that, so I'm not sure how to proceed. The
> >> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
> >> > http://gerrit.cloudera.org:8080/#/c/2992/)
> >> >
> >> > - Wes
> >> >
> >> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <
> emkornfield@gmail.com>
> >> > wrote:
> >> > > Does gerrit work well with TravisCI, or will we need to
> develop/setup
> >> > > another continuous integration solution?
> >> > >
> >> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
> >> > > <da...@gmail.com> wrote:
> >> > >> Admittedly, coming from the complete opposite end of the
> commit-size
> >> > spectrum, the JIRA issue + GitHub pull request workflow already feels
> a
> >> > little frictional for simple bugfixes and additions, so I was wary of
> >> > Gerrit. But it actually looks pretty well-suited to small commits.
> >> > >> One advantage I'd see to different platforms, though, would be the
> >> > potential for JIRA integration. GitHub seems to have a more built-in
> >> > solution for this, if it's something you could foresee setting up. But
> >> > there seem to be ways to do it with Gerrit too.
> >> > >> Clearly having an option to use GitHub pull requests lowers the
> >> > barriers to entry for contributors, but I understand easy pull
> requests
> >> are
> >> > a double-edged sword for maintainers!
> >> > >>
> >> > >>
> >> > >>
> >> > >> _____________________________
> >> > >> From: Wes McKinney <we...@gmail.com>
> >> > >> Sent: Thursday, May 5, 2016 12:46 AM
> >> > >> Subject: Re: Code review tools for Arrow patches
> >> > >> To: <de...@arrow.apache.org>
> >> > >>
> >> > >>
> >> > >> I'm also on board with this if it doesn't deter new contributors
> (it's
> >> > >> a bit of additional process over GitHub but overall not too hard to
> >> > >> learn).
> >> > >>
> >> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <jacques@apache.org
> >
> >> > wrote:
> >> > >>> I dont know about the other pmc members and committers but I
> prefer
> >> > just
> >> > >>> making Gerrit the only way to submit patches rather than one of
> many.
> >> > It
> >> > >>> seems to work well for Asterix and Kudu.
> >> > >>
> >> > >>
> >> > >>
> >> > >>
> >> >
> >>
>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@gmail.com>.
hi folks,
Following up on this. My suggestion for a workflow to help with large
code reviews for Arrow is:
1) We set up an Arrow project on gerrit.cloudera.org. I'm hoping we
see gerrit.apache.org someday.
2) For reviews needing more careful scrutiny, code reviewer can
request to conduct the CR on Gerrit
3) Contributor will push change sets to Gerrit
4) [The slightly awkward part] In parallel, contributor will open a PR
on GitHub for purposes of trigging Travis CI verification
5) Arrow committer may cherry-pick verified commits to master and push
to ASF git repo
My understanding (someone more experienced should chime in) is that
Gerrit reviews are all made relative to the parent commit for a
particular change set. Thus, we may not need to worry (for now) about
synchronization issues between Gerrit and GitHub / ASF git.
Does this make sense? Any other ideas / thoughts welcome
Thanks,
Wes
On Mon, May 23, 2016 at 3:55 PM, Hanifi GUNES <ha...@gmail.com> wrote:
> I worked with Gerrit and GH both. My personal preference would be in favor
> of Gerrit because of its power user ready-ness and tight integration with
> git + git cli. Afaics there are legitimate concerns around possible
> trickiness of novice users' interaction with Gerrit. Not sure if this was
> mentioned above but there seems a Gerrit + GH plugin that mirrors GH
> pull-requests to Gerrit changes. Never used it but still this may be of
> help.
>
>
> 1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
>
> 2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
>
>> If everyone else would prefer Gerrit, I would be okay with using it
>> exclusively to simplify things. It does have several nice features beyond
>> reviewboard as it manages its own git repository, rather than just patch
>> files.
>>
>> Jason Altekruse
>> Software Engineer at Dremio
>> Apache Drill Committer
>>
>> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
>> wrote:
>>
>> > Apparently it is possible, but quite a lot of work:
>> >
>> > https://github.com/andygrunwald/gotrap
>> >
>> > The ideal thing, it would seem, would be to have the Gerrit code
>> > reviews with automatic replication of updated patch sets to a pull
>> > request (i.e. each new patch set force pushes the branch). I don't
>> > think we're going to get that, so I'm not sure how to proceed. The
>> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
>> > http://gerrit.cloudera.org:8080/#/c/2992/)
>> >
>> > - Wes
>> >
>> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <em...@gmail.com>
>> > wrote:
>> > > Does gerrit work well with TravisCI, or will we need to develop/setup
>> > > another continuous integration solution?
>> > >
>> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
>> > > <da...@gmail.com> wrote:
>> > >> Admittedly, coming from the complete opposite end of the commit-size
>> > spectrum, the JIRA issue + GitHub pull request workflow already feels a
>> > little frictional for simple bugfixes and additions, so I was wary of
>> > Gerrit. But it actually looks pretty well-suited to small commits.
>> > >> One advantage I'd see to different platforms, though, would be the
>> > potential for JIRA integration. GitHub seems to have a more built-in
>> > solution for this, if it's something you could foresee setting up. But
>> > there seem to be ways to do it with Gerrit too.
>> > >> Clearly having an option to use GitHub pull requests lowers the
>> > barriers to entry for contributors, but I understand easy pull requests
>> are
>> > a double-edged sword for maintainers!
>> > >>
>> > >>
>> > >>
>> > >> _____________________________
>> > >> From: Wes McKinney <we...@gmail.com>
>> > >> Sent: Thursday, May 5, 2016 12:46 AM
>> > >> Subject: Re: Code review tools for Arrow patches
>> > >> To: <de...@arrow.apache.org>
>> > >>
>> > >>
>> > >> I'm also on board with this if it doesn't deter new contributors (it's
>> > >> a bit of additional process over GitHub but overall not too hard to
>> > >> learn).
>> > >>
>> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org>
>> > wrote:
>> > >>> I dont know about the other pmc members and committers but I prefer
>> > just
>> > >>> making Gerrit the only way to submit patches rather than one of many.
>> > It
>> > >>> seems to work well for Asterix and Kudu.
>> > >>
>> > >>
>> > >>
>> > >>
>> >
>>
Re: Code review tools for Arrow patches
Posted by Hanifi GUNES <ha...@gmail.com>.
I worked with Gerrit and GH both. My personal preference would be in favor
of Gerrit because of its power user ready-ness and tight integration with
git + git cli. Afaics there are legitimate concerns around possible
trickiness of novice users' interaction with Gerrit. Not sure if this was
mentioned above but there seems a Gerrit + GH plugin that mirrors GH
pull-requests to Gerrit changes. Never used it but still this may be of
help.
1: https://gerrit.googlesource.com/plugins/github/+/master/README.md
2016-05-13 8:47 GMT-07:00 Jason Altekruse <ja...@dremio.com>:
> If everyone else would prefer Gerrit, I would be okay with using it
> exclusively to simplify things. It does have several nice features beyond
> reviewboard as it manages its own git repository, rather than just patch
> files.
>
> Jason Altekruse
> Software Engineer at Dremio
> Apache Drill Committer
>
> On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com>
> wrote:
>
> > Apparently it is possible, but quite a lot of work:
> >
> > https://github.com/andygrunwald/gotrap
> >
> > The ideal thing, it would seem, would be to have the Gerrit code
> > reviews with automatic replication of updated patch sets to a pull
> > request (i.e. each new patch set force pushes the branch). I don't
> > think we're going to get that, so I'm not sure how to proceed. The
> > Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
> > http://gerrit.cloudera.org:8080/#/c/2992/)
> >
> > - Wes
> >
> > On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <em...@gmail.com>
> > wrote:
> > > Does gerrit work well with TravisCI, or will we need to develop/setup
> > > another continuous integration solution?
> > >
> > > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
> > > <da...@gmail.com> wrote:
> > >> Admittedly, coming from the complete opposite end of the commit-size
> > spectrum, the JIRA issue + GitHub pull request workflow already feels a
> > little frictional for simple bugfixes and additions, so I was wary of
> > Gerrit. But it actually looks pretty well-suited to small commits.
> > >> One advantage I'd see to different platforms, though, would be the
> > potential for JIRA integration. GitHub seems to have a more built-in
> > solution for this, if it's something you could foresee setting up. But
> > there seem to be ways to do it with Gerrit too.
> > >> Clearly having an option to use GitHub pull requests lowers the
> > barriers to entry for contributors, but I understand easy pull requests
> are
> > a double-edged sword for maintainers!
> > >>
> > >>
> > >>
> > >> _____________________________
> > >> From: Wes McKinney <we...@gmail.com>
> > >> Sent: Thursday, May 5, 2016 12:46 AM
> > >> Subject: Re: Code review tools for Arrow patches
> > >> To: <de...@arrow.apache.org>
> > >>
> > >>
> > >> I'm also on board with this if it doesn't deter new contributors (it's
> > >> a bit of additional process over GitHub but overall not too hard to
> > >> learn).
> > >>
> > >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> > >>> I dont know about the other pmc members and committers but I prefer
> > just
> > >>> making Gerrit the only way to submit patches rather than one of many.
> > It
> > >>> seems to work well for Asterix and Kudu.
> > >>
> > >>
> > >>
> > >>
> >
>
Re: Code review tools for Arrow patches
Posted by Jason Altekruse <ja...@dremio.com>.
If everyone else would prefer Gerrit, I would be okay with using it
exclusively to simplify things. It does have several nice features beyond
reviewboard as it manages its own git repository, rather than just patch
files.
Jason Altekruse
Software Engineer at Dremio
Apache Drill Committer
On Thu, May 12, 2016 at 10:03 PM, Wes McKinney <we...@gmail.com> wrote:
> Apparently it is possible, but quite a lot of work:
>
> https://github.com/andygrunwald/gotrap
>
> The ideal thing, it would seem, would be to have the Gerrit code
> reviews with automatic replication of updated patch sets to a pull
> request (i.e. each new patch set force pushes the branch). I don't
> think we're going to get that, so I'm not sure how to proceed. The
> Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
> http://gerrit.cloudera.org:8080/#/c/2992/)
>
> - Wes
>
> On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <em...@gmail.com>
> wrote:
> > Does gerrit work well with TravisCI, or will we need to develop/setup
> > another continuous integration solution?
> >
> > On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
> > <da...@gmail.com> wrote:
> >> Admittedly, coming from the complete opposite end of the commit-size
> spectrum, the JIRA issue + GitHub pull request workflow already feels a
> little frictional for simple bugfixes and additions, so I was wary of
> Gerrit. But it actually looks pretty well-suited to small commits.
> >> One advantage I'd see to different platforms, though, would be the
> potential for JIRA integration. GitHub seems to have a more built-in
> solution for this, if it's something you could foresee setting up. But
> there seem to be ways to do it with Gerrit too.
> >> Clearly having an option to use GitHub pull requests lowers the
> barriers to entry for contributors, but I understand easy pull requests are
> a double-edged sword for maintainers!
> >>
> >>
> >>
> >> _____________________________
> >> From: Wes McKinney <we...@gmail.com>
> >> Sent: Thursday, May 5, 2016 12:46 AM
> >> Subject: Re: Code review tools for Arrow patches
> >> To: <de...@arrow.apache.org>
> >>
> >>
> >> I'm also on board with this if it doesn't deter new contributors (it's
> >> a bit of additional process over GitHub but overall not too hard to
> >> learn).
> >>
> >> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
> >>> I dont know about the other pmc members and committers but I prefer
> just
> >>> making Gerrit the only way to submit patches rather than one of many.
> It
> >>> seems to work well for Asterix and Kudu.
> >>
> >>
> >>
> >>
>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@gmail.com>.
Apparently it is possible, but quite a lot of work:
https://github.com/andygrunwald/gotrap
The ideal thing, it would seem, would be to have the Gerrit code
reviews with automatic replication of updated patch sets to a pull
request (i.e. each new patch set force pushes the branch). I don't
think we're going to get that, so I'm not sure how to proceed. The
Kudu team uses Gerrit + Jenkins trigger (e.g. see it in action here
http://gerrit.cloudera.org:8080/#/c/2992/)
- Wes
On Mon, May 9, 2016 at 4:48 PM, Micah Kornfield <em...@gmail.com> wrote:
> Does gerrit work well with TravisCI, or will we need to develop/setup
> another continuous integration solution?
>
> On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
> <da...@gmail.com> wrote:
>> Admittedly, coming from the complete opposite end of the commit-size spectrum, the JIRA issue + GitHub pull request workflow already feels a little frictional for simple bugfixes and additions, so I was wary of Gerrit. But it actually looks pretty well-suited to small commits.
>> One advantage I'd see to different platforms, though, would be the potential for JIRA integration. GitHub seems to have a more built-in solution for this, if it's something you could foresee setting up. But there seem to be ways to do it with Gerrit too.
>> Clearly having an option to use GitHub pull requests lowers the barriers to entry for contributors, but I understand easy pull requests are a double-edged sword for maintainers!
>>
>>
>>
>> _____________________________
>> From: Wes McKinney <we...@gmail.com>
>> Sent: Thursday, May 5, 2016 12:46 AM
>> Subject: Re: Code review tools for Arrow patches
>> To: <de...@arrow.apache.org>
>>
>>
>> I'm also on board with this if it doesn't deter new contributors (it's
>> a bit of additional process over GitHub but overall not too hard to
>> learn).
>>
>> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org> wrote:
>>> I dont know about the other pmc members and committers but I prefer just
>>> making Gerrit the only way to submit patches rather than one of many. It
>>> seems to work well for Asterix and Kudu.
>>
>>
>>
>>
Re: Code review tools for Arrow patches
Posted by Micah Kornfield <em...@gmail.com>.
Does gerrit work well with TravisCI, or will we need to develop/setup
another continuous integration solution?
On Wed, May 4, 2016 at 10:08 PM, Daniel Robinson
<da...@gmail.com> wrote:
> Admittedly, coming from the complete opposite end of the commit-size spectrum, the JIRA issue + GitHub pull request workflow already feels a little frictional for simple bugfixes and additions, so I was wary of Gerrit. But it actually looks pretty well-suited to small commits.
> One advantage I'd see to different platforms, though, would be the potential for JIRA integration. GitHub seems to have a more built-in solution for this, if it's something you could foresee setting up. But there seem to be ways to do it with Gerrit too.
> Clearly having an option to use GitHub pull requests lowers the barriers to entry for contributors, but I understand easy pull requests are a double-edged sword for maintainers!
>
>
>
> _____________________________
> From: Wes McKinney <we...@gmail.com>
> Sent: Thursday, May 5, 2016 12:46 AM
> Subject: Re: Code review tools for Arrow patches
> To: <de...@arrow.apache.org>
>
>
> I'm also on board with this if it doesn't deter new contributors (it's
> a bit of additional process over GitHub but overall not too hard to
> learn).
>
> On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org> wrote:
>> I dont know about the other pmc members and committers but I prefer just
>> making Gerrit the only way to submit patches rather than one of many. It
>> seems to work well for Asterix and Kudu.
>
>
>
>
Re: Code review tools for Arrow patches
Posted by Daniel Robinson <da...@gmail.com>.
Admittedly, coming from the complete opposite end of the commit-size spectrum, the JIRA issue + GitHub pull request workflow already feels a little frictional for simple bugfixes and additions, so I was wary of Gerrit. But it actually looks pretty well-suited to small commits.
One advantage I'd see to different platforms, though, would be the potential for JIRA integration. GitHub seems to have a more built-in solution for this, if it's something you could foresee setting up. But there seem to be ways to do it with Gerrit too.
Clearly having an option to use GitHub pull requests lowers the barriers to entry for contributors, but I understand easy pull requests are a double-edged sword for maintainers!
_____________________________
From: Wes McKinney <we...@gmail.com>
Sent: Thursday, May 5, 2016 12:46 AM
Subject: Re: Code review tools for Arrow patches
To: <de...@arrow.apache.org>
I'm also on board with this if it doesn't deter new contributors (it's
a bit of additional process over GitHub but overall not too hard to
learn).
On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org> wrote:
> I dont know about the other pmc members and committers but I prefer just
> making Gerrit the only way to submit patches rather than one of many. It
> seems to work well for Asterix and Kudu.
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@gmail.com>.
I'm also on board with this if it doesn't deter new contributors (it's
a bit of additional process over GitHub but overall not too hard to
learn).
On Tue, May 3, 2016 at 9:59 AM, Jacques Nadeau <ja...@apache.org> wrote:
> I dont know about the other pmc members and committers but I prefer just
> making Gerrit the only way to submit patches rather than one of many. It
> seems to work well for Asterix and Kudu.
Re: Code review tools for Arrow patches
Posted by Jacques Nadeau <ja...@apache.org>.
I dont know about the other pmc members and committers but I prefer just
making Gerrit the only way to submit patches rather than one of many. It
seems to work well for Asterix and Kudu.
On Apr 25, 2016 12:16 PM, "Todd Lipcon" <to...@cloudera.com> wrote:
> On Mon, Apr 25, 2016 at 12:09 PM, Wes McKinney <we...@cloudera.com> wrote:
>
> > hi Todd,
> >
> > This is helpful to know, thank you. As you say, by "optional" what I
> > meant was that code could be optionally reviewed in Gerrit, but then
> > commits would have to be cherry-picked by the committer from the
> > Gerrit git remote. We would continue to accept patches via GitHub pull
> > requests (but for larger patches, they may move from GitHub to gerrit
> > as need be). Unless there is some pitfall here that I'm missing.
> >
>
> The pitfall is what the state of the gerrit remote looks like. How do you
> keep it up to date with the ASF repo, if you have patches entering the ASF
> repo from some mechanism other than gerrit?
>
> It might be possible involving some cron job which force pushes from ASF ->
> Gerrit, but I haven't ever tried a workflow like that.
>
> -Todd
>
>
> >
> > On Mon, Apr 25, 2016 at 3:03 PM, Todd Lipcon <to...@cloudera.com> wrote:
> > > Using gerrit as an "optional" tool is a bit difficult, because it
> doesn't
> > > know how to handle commits to a repository that it doesn't own.
> > >
> > > The way we get around the "commit via gerrit" issue in the Kudu podling
> > is
> > > to follow the example of AsterixDB. Commits are made using gerrit, but
> > that
> > > doesn't automatically flow to the ASF repo. The committer then runs a
> > > 'push-to-asf.py' script which grabs the commit from gerrit and pushes
> to
> > > the ASF repository:
> > >
> >
> https://github.com/apache/incubator-kudu/blob/master/build-support/push_to_asf.py
> > >
> > > I'm happy to set up the gerrit projects, but not sure how it would work
> > in
> > > an "optional" context.
> > >
> > > -Todd
> > >
> > >
> > > On Sun, Apr 24, 2016 at 4:53 PM, Julian Hyde <jh...@gmail.com>
> > wrote:
> > >
> > >> IIRC Apex wanted to commit via Gerrit. That was a non-starter. Commits
> > >> have to be made by a committer.
> > >>
> > >> Julian
> > >>
> > >>
> > >> > On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com> wrote:
> > >> >
> > >> > Sending all Gerrit review activity to the mailing list seems
> adequate
> > to
> > >> me.
> > >> > I don't see how this is especially different from reviewing code on
> a
> > >> > website owned by GitHub. I remain hopeful that ASF Infra will set up
> > an
> > >> > ASF-managed Gerrit.
> > >> >
> > >> > On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com>
> wrote:
> > >> >
> > >> >> Just for the record, Apex had some issues getting Gerrit reviews
> > >> reflected
> > >> >> in a coherent fashion into the Apache record. I presume that you
> guys
> > >> will
> > >> >> have that handled or will check with the Apex devs to learn their
> > >> >> resolution.
> > >> >>
> > >> >>
> > >> >>
> > >>
> > >>
> > >
> > >
> > > --
> > > Todd Lipcon
> > > Software Engineer, Cloudera
> >
>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>
Re: Code review tools for Arrow patches
Posted by Todd Lipcon <to...@cloudera.com>.
On Mon, Apr 25, 2016 at 12:09 PM, Wes McKinney <we...@cloudera.com> wrote:
> hi Todd,
>
> This is helpful to know, thank you. As you say, by "optional" what I
> meant was that code could be optionally reviewed in Gerrit, but then
> commits would have to be cherry-picked by the committer from the
> Gerrit git remote. We would continue to accept patches via GitHub pull
> requests (but for larger patches, they may move from GitHub to gerrit
> as need be). Unless there is some pitfall here that I'm missing.
>
The pitfall is what the state of the gerrit remote looks like. How do you
keep it up to date with the ASF repo, if you have patches entering the ASF
repo from some mechanism other than gerrit?
It might be possible involving some cron job which force pushes from ASF ->
Gerrit, but I haven't ever tried a workflow like that.
-Todd
>
> On Mon, Apr 25, 2016 at 3:03 PM, Todd Lipcon <to...@cloudera.com> wrote:
> > Using gerrit as an "optional" tool is a bit difficult, because it doesn't
> > know how to handle commits to a repository that it doesn't own.
> >
> > The way we get around the "commit via gerrit" issue in the Kudu podling
> is
> > to follow the example of AsterixDB. Commits are made using gerrit, but
> that
> > doesn't automatically flow to the ASF repo. The committer then runs a
> > 'push-to-asf.py' script which grabs the commit from gerrit and pushes to
> > the ASF repository:
> >
> https://github.com/apache/incubator-kudu/blob/master/build-support/push_to_asf.py
> >
> > I'm happy to set up the gerrit projects, but not sure how it would work
> in
> > an "optional" context.
> >
> > -Todd
> >
> >
> > On Sun, Apr 24, 2016 at 4:53 PM, Julian Hyde <jh...@gmail.com>
> wrote:
> >
> >> IIRC Apex wanted to commit via Gerrit. That was a non-starter. Commits
> >> have to be made by a committer.
> >>
> >> Julian
> >>
> >>
> >> > On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com> wrote:
> >> >
> >> > Sending all Gerrit review activity to the mailing list seems adequate
> to
> >> me.
> >> > I don't see how this is especially different from reviewing code on a
> >> > website owned by GitHub. I remain hopeful that ASF Infra will set up
> an
> >> > ASF-managed Gerrit.
> >> >
> >> > On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com> wrote:
> >> >
> >> >> Just for the record, Apex had some issues getting Gerrit reviews
> >> reflected
> >> >> in a coherent fashion into the Apache record. I presume that you guys
> >> will
> >> >> have that handled or will check with the Apex devs to learn their
> >> >> resolution.
> >> >>
> >> >>
> >> >>
> >>
> >>
> >
> >
> > --
> > Todd Lipcon
> > Software Engineer, Cloudera
>
--
Todd Lipcon
Software Engineer, Cloudera
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@cloudera.com>.
hi Todd,
This is helpful to know, thank you. As you say, by "optional" what I
meant was that code could be optionally reviewed in Gerrit, but then
commits would have to be cherry-picked by the committer from the
Gerrit git remote. We would continue to accept patches via GitHub pull
requests (but for larger patches, they may move from GitHub to gerrit
as need be). Unless there is some pitfall here that I'm missing.
Thanks
Wes
On Mon, Apr 25, 2016 at 3:03 PM, Todd Lipcon <to...@cloudera.com> wrote:
> Using gerrit as an "optional" tool is a bit difficult, because it doesn't
> know how to handle commits to a repository that it doesn't own.
>
> The way we get around the "commit via gerrit" issue in the Kudu podling is
> to follow the example of AsterixDB. Commits are made using gerrit, but that
> doesn't automatically flow to the ASF repo. The committer then runs a
> 'push-to-asf.py' script which grabs the commit from gerrit and pushes to
> the ASF repository:
> https://github.com/apache/incubator-kudu/blob/master/build-support/push_to_asf.py
>
> I'm happy to set up the gerrit projects, but not sure how it would work in
> an "optional" context.
>
> -Todd
>
>
> On Sun, Apr 24, 2016 at 4:53 PM, Julian Hyde <jh...@gmail.com> wrote:
>
>> IIRC Apex wanted to commit via Gerrit. That was a non-starter. Commits
>> have to be made by a committer.
>>
>> Julian
>>
>>
>> > On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com> wrote:
>> >
>> > Sending all Gerrit review activity to the mailing list seems adequate to
>> me.
>> > I don't see how this is especially different from reviewing code on a
>> > website owned by GitHub. I remain hopeful that ASF Infra will set up an
>> > ASF-managed Gerrit.
>> >
>> > On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com> wrote:
>> >
>> >> Just for the record, Apex had some issues getting Gerrit reviews
>> reflected
>> >> in a coherent fashion into the Apache record. I presume that you guys
>> will
>> >> have that handled or will check with the Apex devs to learn their
>> >> resolution.
>> >>
>> >>
>> >>
>>
>>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
Re: Code review tools for Arrow patches
Posted by Todd Lipcon <to...@cloudera.com>.
Using gerrit as an "optional" tool is a bit difficult, because it doesn't
know how to handle commits to a repository that it doesn't own.
The way we get around the "commit via gerrit" issue in the Kudu podling is
to follow the example of AsterixDB. Commits are made using gerrit, but that
doesn't automatically flow to the ASF repo. The committer then runs a
'push-to-asf.py' script which grabs the commit from gerrit and pushes to
the ASF repository:
https://github.com/apache/incubator-kudu/blob/master/build-support/push_to_asf.py
I'm happy to set up the gerrit projects, but not sure how it would work in
an "optional" context.
-Todd
On Sun, Apr 24, 2016 at 4:53 PM, Julian Hyde <jh...@gmail.com> wrote:
> IIRC Apex wanted to commit via Gerrit. That was a non-starter. Commits
> have to be made by a committer.
>
> Julian
>
>
> > On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com> wrote:
> >
> > Sending all Gerrit review activity to the mailing list seems adequate to
> me.
> > I don't see how this is especially different from reviewing code on a
> > website owned by GitHub. I remain hopeful that ASF Infra will set up an
> > ASF-managed Gerrit.
> >
> > On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com> wrote:
> >
> >> Just for the record, Apex had some issues getting Gerrit reviews
> reflected
> >> in a coherent fashion into the Apache record. I presume that you guys
> will
> >> have that handled or will check with the Apex devs to learn their
> >> resolution.
> >>
> >>
> >>
>
>
--
Todd Lipcon
Software Engineer, Cloudera
Re: Code review tools for Arrow patches
Posted by Julian Hyde <jh...@gmail.com>.
IIRC Apex wanted to commit via Gerrit. That was a non-starter. Commits have to be made by a committer.
Julian
> On Apr 24, 2016, at 3:07 PM, Wes McKinney <we...@cloudera.com> wrote:
>
> Sending all Gerrit review activity to the mailing list seems adequate to me.
> I don't see how this is especially different from reviewing code on a
> website owned by GitHub. I remain hopeful that ASF Infra will set up an
> ASF-managed Gerrit.
>
> On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com> wrote:
>
>> Just for the record, Apex had some issues getting Gerrit reviews reflected
>> in a coherent fashion into the Apache record. I presume that you guys will
>> have that handled or will check with the Apex devs to learn their
>> resolution.
>>
>>
>>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@cloudera.com>.
Sending all Gerrit review activity to the mailing list seems adequate to me.
I don't see how this is especially different from reviewing code on a
website owned by GitHub. I remain hopeful that ASF Infra will set up an
ASF-managed Gerrit.
On Sunday, April 24, 2016, Ted Dunning <te...@gmail.com> wrote:
> Just for the record, Apex had some issues getting Gerrit reviews reflected
> in a coherent fashion into the Apache record. I presume that you guys will
> have that handled or will check with the Apex devs to learn their
> resolution.
>
>
>
Re: Code review tools for Arrow patches
Posted by Ted Dunning <te...@gmail.com>.
Just for the record, Apex had some issues getting Gerrit reviews reflected
in a coherent fashion into the Apache record. I presume that you guys will
have that handled or will check with the Apex devs to learn their
resolution.
On Sun, Apr 24, 2016 at 5:30 PM, Wes McKinney <we...@cloudera.com> wrote:
> Todd, would you (or someone who is familiar with Gerrit
> administration) mind setting up gerrit.cloudera.org projects that we
> can use for apache/arrow and apache/parquet-cpp? We're having more
> larger patches to both projects and I'm having a difficult time
> staying organized in my reviews on GitHub (especially reviewing
> updated patch sets). I'll provide instructions for using these
> instances as an optional review tool (will definitely not be required
> as part of the patch sign-off procedure).
>
> Thank you
> Wes
>
> On Fri, Apr 15, 2016 at 3:48 PM, Wes McKinney <we...@cloudera.com> wrote:
> > hey Jason,
> >
> > I have not used Reviewboard, but the problems you are describing are
> > (AFAICT) not common complaint among Gerrit users. It would be helpful
> > to hear more from experienced Gerrit users.
> >
> > Note that my initial request was not "let's choose a tool that is not
> > GitHub PRs" for code reviews but rather that "let's have more tools"
> > -- I'm more than happy if groups of committers choose the tool that
> > works best for them, and we proceed in a somewhat anarchic fashion for
> > the time being. I think using PRs as training wheels for newcomers is
> > perfectly acceptable and when the patches grow more extensive,
> > reviewers can make a judgment call when using a more advanced tool (I
> > would describe Gerrit as requiring somewhat advanced git expertise,
> > rebasing and so forth) makes sense.
> >
> > - Wes
> >
> > On Fri, Apr 15, 2016 at 7:34 PM, Jason Altekruse
> > <al...@gmail.com> wrote:
> >> It looks like I am going to be a minority opinion here, but I think
> there
> >> is at least a case to make that pull requests area little easier for
> >> newcomers.
> >> I also have opinions about rebasing branches that are shared publicly or
> >> currently under review. While it isn't often a problem, rebasing often
> >> involves squashing which further distorts the history of how a feature
> was
> >> developed. This is part of the reason why we had moved to Pull requests
> for
> >> Drill. We reached a juncture where we were making fixes and implementing
> >> features often required refactoring or general cleanup, and the reviews
> got
> >> messy on reviewboard. Pull requests make it easier to review a
> multi-part
> >> changeset, where cosmetic or refactoring changes can be in separate
> commit
> >> from bug fixes or features. The github interface does give you decent
> >> access to view individual commits, or the combination of all of the
> work on
> >> the branch.
> >>
> >> I have only been working with gerrit recently, but it seems to have the
> >> same behavior or reviewboard in terms of providing a mechanism for
> looking
> >> at essentially diffs between diffs. The relationship between these
> overall
> >> changeset revisions can get quite complicated, as the first one will be
> >> based off an earlier version of master, then an update will be rebased
> on
> >> the lastest master, with the original work along with any conflict
> >> resolution smashed into the same unit. This always seemed a bit
> >> counter-intuitive to me, because it puts information about the project
> >> history that could be stored in git, using merge commits, instead
> inside of
> >> the database of the web app.
> >>
> >> - Jason
> >>
> >> On Fri, Apr 15, 2016 at 10:08 AM, Todd Lipcon <to...@cloudera.com>
> wrote:
> >>
> >>> +1 for Gerrit from me too -- we're using it on Kudu and everyone on the
> >>> team likes it.
> >>>
> >>> Happy to help admin the server on the gerrit.cloudera.org box which
> hosts
> >>> Kudu and Impala
> >>>
> >>> -Todd
> >>>
> >>> On Fri, Apr 15, 2016 at 8:48 AM, Wes McKinney <we...@cloudera.com>
> wrote:
> >>>
> >>> > 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 <ka...@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
> >>> > >
> >>> >
> >>>
> >>>
> >>>
> >>> --
> >>> Todd Lipcon
> >>> Software Engineer, Cloudera
> >>>
>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@cloudera.com>.
Todd, would you (or someone who is familiar with Gerrit
administration) mind setting up gerrit.cloudera.org projects that we
can use for apache/arrow and apache/parquet-cpp? We're having more
larger patches to both projects and I'm having a difficult time
staying organized in my reviews on GitHub (especially reviewing
updated patch sets). I'll provide instructions for using these
instances as an optional review tool (will definitely not be required
as part of the patch sign-off procedure).
Thank you
Wes
On Fri, Apr 15, 2016 at 3:48 PM, Wes McKinney <we...@cloudera.com> wrote:
> hey Jason,
>
> I have not used Reviewboard, but the problems you are describing are
> (AFAICT) not common complaint among Gerrit users. It would be helpful
> to hear more from experienced Gerrit users.
>
> Note that my initial request was not "let's choose a tool that is not
> GitHub PRs" for code reviews but rather that "let's have more tools"
> -- I'm more than happy if groups of committers choose the tool that
> works best for them, and we proceed in a somewhat anarchic fashion for
> the time being. I think using PRs as training wheels for newcomers is
> perfectly acceptable and when the patches grow more extensive,
> reviewers can make a judgment call when using a more advanced tool (I
> would describe Gerrit as requiring somewhat advanced git expertise,
> rebasing and so forth) makes sense.
>
> - Wes
>
> On Fri, Apr 15, 2016 at 7:34 PM, Jason Altekruse
> <al...@gmail.com> wrote:
>> It looks like I am going to be a minority opinion here, but I think there
>> is at least a case to make that pull requests area little easier for
>> newcomers.
>> I also have opinions about rebasing branches that are shared publicly or
>> currently under review. While it isn't often a problem, rebasing often
>> involves squashing which further distorts the history of how a feature was
>> developed. This is part of the reason why we had moved to Pull requests for
>> Drill. We reached a juncture where we were making fixes and implementing
>> features often required refactoring or general cleanup, and the reviews got
>> messy on reviewboard. Pull requests make it easier to review a multi-part
>> changeset, where cosmetic or refactoring changes can be in separate commit
>> from bug fixes or features. The github interface does give you decent
>> access to view individual commits, or the combination of all of the work on
>> the branch.
>>
>> I have only been working with gerrit recently, but it seems to have the
>> same behavior or reviewboard in terms of providing a mechanism for looking
>> at essentially diffs between diffs. The relationship between these overall
>> changeset revisions can get quite complicated, as the first one will be
>> based off an earlier version of master, then an update will be rebased on
>> the lastest master, with the original work along with any conflict
>> resolution smashed into the same unit. This always seemed a bit
>> counter-intuitive to me, because it puts information about the project
>> history that could be stored in git, using merge commits, instead inside of
>> the database of the web app.
>>
>> - Jason
>>
>> On Fri, Apr 15, 2016 at 10:08 AM, Todd Lipcon <to...@cloudera.com> wrote:
>>
>>> +1 for Gerrit from me too -- we're using it on Kudu and everyone on the
>>> team likes it.
>>>
>>> Happy to help admin the server on the gerrit.cloudera.org box which hosts
>>> Kudu and Impala
>>>
>>> -Todd
>>>
>>> On Fri, Apr 15, 2016 at 8:48 AM, Wes McKinney <we...@cloudera.com> wrote:
>>>
>>> > 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 <ka...@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
>>> > >
>>> >
>>>
>>>
>>>
>>> --
>>> Todd Lipcon
>>> Software Engineer, Cloudera
>>>
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@cloudera.com>.
hey Jason,
I have not used Reviewboard, but the problems you are describing are
(AFAICT) not common complaint among Gerrit users. It would be helpful
to hear more from experienced Gerrit users.
Note that my initial request was not "let's choose a tool that is not
GitHub PRs" for code reviews but rather that "let's have more tools"
-- I'm more than happy if groups of committers choose the tool that
works best for them, and we proceed in a somewhat anarchic fashion for
the time being. I think using PRs as training wheels for newcomers is
perfectly acceptable and when the patches grow more extensive,
reviewers can make a judgment call when using a more advanced tool (I
would describe Gerrit as requiring somewhat advanced git expertise,
rebasing and so forth) makes sense.
- Wes
On Fri, Apr 15, 2016 at 7:34 PM, Jason Altekruse
<al...@gmail.com> wrote:
> It looks like I am going to be a minority opinion here, but I think there
> is at least a case to make that pull requests area little easier for
> newcomers.
> I also have opinions about rebasing branches that are shared publicly or
> currently under review. While it isn't often a problem, rebasing often
> involves squashing which further distorts the history of how a feature was
> developed. This is part of the reason why we had moved to Pull requests for
> Drill. We reached a juncture where we were making fixes and implementing
> features often required refactoring or general cleanup, and the reviews got
> messy on reviewboard. Pull requests make it easier to review a multi-part
> changeset, where cosmetic or refactoring changes can be in separate commit
> from bug fixes or features. The github interface does give you decent
> access to view individual commits, or the combination of all of the work on
> the branch.
>
> I have only been working with gerrit recently, but it seems to have the
> same behavior or reviewboard in terms of providing a mechanism for looking
> at essentially diffs between diffs. The relationship between these overall
> changeset revisions can get quite complicated, as the first one will be
> based off an earlier version of master, then an update will be rebased on
> the lastest master, with the original work along with any conflict
> resolution smashed into the same unit. This always seemed a bit
> counter-intuitive to me, because it puts information about the project
> history that could be stored in git, using merge commits, instead inside of
> the database of the web app.
>
> - Jason
>
> On Fri, Apr 15, 2016 at 10:08 AM, Todd Lipcon <to...@cloudera.com> wrote:
>
>> +1 for Gerrit from me too -- we're using it on Kudu and everyone on the
>> team likes it.
>>
>> Happy to help admin the server on the gerrit.cloudera.org box which hosts
>> Kudu and Impala
>>
>> -Todd
>>
>> On Fri, Apr 15, 2016 at 8:48 AM, Wes McKinney <we...@cloudera.com> wrote:
>>
>> > 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 <ka...@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
>> > >
>> >
>>
>>
>>
>> --
>> Todd Lipcon
>> Software Engineer, Cloudera
>>
Re: Code review tools for Arrow patches
Posted by Jason Altekruse <al...@gmail.com>.
It looks like I am going to be a minority opinion here, but I think there
is at least a case to make that pull requests area little easier for
newcomers.
I also have opinions about rebasing branches that are shared publicly or
currently under review. While it isn't often a problem, rebasing often
involves squashing which further distorts the history of how a feature was
developed. This is part of the reason why we had moved to Pull requests for
Drill. We reached a juncture where we were making fixes and implementing
features often required refactoring or general cleanup, and the reviews got
messy on reviewboard. Pull requests make it easier to review a multi-part
changeset, where cosmetic or refactoring changes can be in separate commit
from bug fixes or features. The github interface does give you decent
access to view individual commits, or the combination of all of the work on
the branch.
I have only been working with gerrit recently, but it seems to have the
same behavior or reviewboard in terms of providing a mechanism for looking
at essentially diffs between diffs. The relationship between these overall
changeset revisions can get quite complicated, as the first one will be
based off an earlier version of master, then an update will be rebased on
the lastest master, with the original work along with any conflict
resolution smashed into the same unit. This always seemed a bit
counter-intuitive to me, because it puts information about the project
history that could be stored in git, using merge commits, instead inside of
the database of the web app.
- Jason
On Fri, Apr 15, 2016 at 10:08 AM, Todd Lipcon <to...@cloudera.com> wrote:
> +1 for Gerrit from me too -- we're using it on Kudu and everyone on the
> team likes it.
>
> Happy to help admin the server on the gerrit.cloudera.org box which hosts
> Kudu and Impala
>
> -Todd
>
> On Fri, Apr 15, 2016 at 8:48 AM, Wes McKinney <we...@cloudera.com> wrote:
>
> > 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 <ka...@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
> > >
> >
>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>
Re: Code review tools for Arrow patches
Posted by Todd Lipcon <to...@cloudera.com>.
+1 for Gerrit from me too -- we're using it on Kudu and everyone on the
team likes it.
Happy to help admin the server on the gerrit.cloudera.org box which hosts
Kudu and Impala
-Todd
On Fri, Apr 15, 2016 at 8:48 AM, Wes McKinney <we...@cloudera.com> wrote:
> 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 <ka...@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
> >
>
--
Todd Lipcon
Software Engineer, Cloudera
Re: Code review tools for Arrow patches
Posted by Wes McKinney <we...@cloudera.com>.
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 <ka...@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
>
RE: Code review tools for Arrow patches
Posted by "Zheng, Kai" <ka...@intel.com>.
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]
Sent: Friday, April 15, 2016 1:38 AM
To: dev@arrow.apache.org
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
Re: Code review tools for Arrow patches
Posted by "Wang, Yanping" <ya...@intel.com>.
I agree, we need a code review tools as well for Mnemonic. We are going to submit large patches. I include mnemonic dev list here for discussion.
Thanks
> On Apr 14, 2016, at 10:39 AM, Wes McKinney <we...@cloudera.com> wrote:
>
> 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