You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Jacques Nadeau <ja...@apache.org> on 2016/05/03 15:59:52 UTC

Re: Code review tools for Arrow patches

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 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 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
> >
>