You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jason Altekruse <al...@gmail.com> on 2015/06/22 22:02:17 UTC

[DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Hello Drill developers,

I am writing this message today to propose allowing the use of github pull
requests to perform reviews in place of the apache reviewboard instance.

Reviewboard has caused a number of headaches in the past few months, and I
think its time to evaluate the benefits of the apache infrastructure
relative to the actual cost of using it in practice.

For clarity of the discussion, we cannot use the complete github workflow.
Comitters will still need to use patch files, or check out the branch used
in the review request and push to apache master manually. I am not
advocating for using a merging strategy with git, just for using the github
web UI for reviews. I expect anyone generating a chain of commits as
described below to use the rebasing workflow we do today. Additionally devs
should only be breaking up work to make it easier to review, we will not be
reviewing branches that contain a bunch of useless WIP commits.

A few examples of problems I have experienced with reviewboard include:
corruption of patches when they are downloaded, the web interface showing
inconsistent content from the raw diff, and random rejection of patches
that are based directly on the head of apache master.

These are all serious blockers for getting code reviewed and integrated
into the master branch in a timely manner.

In addition to serious bugs in reviewboard, there are a number of
difficulties with the combination of our typical dev workflow and how
reviewboard works with patches. As we are still adding features to Drill,
we often have several weeks of work to submit in response to a JIRA or
series of related JIRAs. Sometimes this work can be broken up into
independent reviewable units, and other times it cannot. When a series of
changes requires a mixture of refactoring and additions, the process is
currently quite painful. Ether reviewers need to look through a giant messy
diff, or the submitters need to do a lot of extra work. This involves not
only organizing their work into a reviewable series of commits, but also
generating redundant squashed versions of the intermediate work to make
reviewboard happy.

For a relatively simple 3 part change, this involves creating 3 reviewboard
pages. The first will contain the first commit by itself. The second will
have the first commits patch as a parent patch with the next change in the
series uploaded as the core change to review. For the third change, a
squashed version of the first two commits must be generated to serve as a
parent patch and then the third changeset uploaded as the reviewable
change. Frequently a change to the first commit requires regenerating all
of these patches and uploading them to the individual review pages.

This gets even worse with larger chains of commits.

It would be great if all of our changes could be small units of work, but
very frequently we want to make sure we are ready to merge a complete
feature before starting the review process. We need to have a better way to
manage these large review units, as I do not see the possibility of
breaking up the work into smaller units as a likely solution. We still have
lots of features and system cleanup to work on.

For anyone unfamiliar, github pull requests are based on a branch you push
to your personal fork. They give space for a general discussion, as well as
allow commenting inline on the diff. They give a clear reference to each
commit in the branch, allowing reviewers to see each piece of work
individually as well as provide a "squashed" view to see the overall
differences.

For the sake of keeping the project history connected to JIRA, we can see
if there is enough automatic github integration or possibly upload patch
files to JIRA each time we update a pull request. As an side note, if we
don't need individual patches for reviewboard we could just put patch files
on JIRA that contain several commits. These are much easier to generate an
apply than a bunch of individual files for each change. This should prevent
JIRAs needing long lists of patches with names like
DRILL-3000-part1-version3.patch

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jason Altekruse <al...@gmail.com>.
Proposed process below in the linked google doc. Please make suggestions in
comments.

Much of the content is a description of the current process, which the dev
team is already familiar with. I'm hoping that this will be detailed enough
that we can just include it in the docs as the general guidelines for
submitting a patch to Drill.

https://docs.google.com/document/d/1vwwBZCIQZFs97PEYq6XEwNu4z512OxmBhIXC5cJQva4


On Tue, Jul 7, 2015 at 2:48 PM, Parth Chandra <pa...@apache.org> wrote:

> I would suggest we start now. Except the the ones that are already out
> there on RB, let's see if we can move to pull requests starting now.
>
> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org> wrote:
>
> > What did we decide here?
> >
> > Are we going to move forward with trying out pull requests?  If so, do we
> > want to start having everyone do it or suggest only one or two do it to
> > start?
> >
> > thoughts?
> > Jacques
> >
> > On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> >
> > > I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> order
> > > to get under 3 gb, I think we need to run single threaded.  Last I
> > checked,
> > > running with 4 threads on dedicated hardware completes in ~12 minutes.
> > > However, the Travis instances used to be really slow virtual machines.
> > I'm
> > > sure a solution can be found but I think we'd need some concerted
> effort
> > on
> > > reducing the test footprint.
> > >
> > > We talked before (Daniel's suggestion) about treating more of the tests
> > as
> > > integration tests.  This would help as much of the test time is spent
> > > starting and stopping Drillbits for each test class.  If we only did
> this
> > > once for all those tests, the footprint would be much smaller.
> > >
> > >
> > >
> > > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> > > wrote:
> > >
> > >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
> > >> wrote:
> > >>
> > >> > We tried Travis before.  The problem is that travis's nodes aren't
> > >> > substantial enough to complete our test suite within their timeout.
> > >> >
> > >>
> > >> Haven't the tests been substantially improved since then?
> > >>
> > >> Can the tests be segregated into pieces so Travis can still do some
> > useful
> > >> work?
> > >>
> > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Parth Chandra <pa...@apache.org>.
I would suggest we start now. Except the the ones that are already out
there on RB, let's see if we can move to pull requests starting now.

On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org> wrote:

> What did we decide here?
>
> Are we going to move forward with trying out pull requests?  If so, do we
> want to start having everyone do it or suggest only one or two do it to
> start?
>
> thoughts?
> Jacques
>
> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
>
> > I can't remember the rule.  I think it was 3gb and 15 minutes.  In order
> > to get under 3 gb, I think we need to run single threaded.  Last I
> checked,
> > running with 4 threads on dedicated hardware completes in ~12 minutes.
> > However, the Travis instances used to be really slow virtual machines.
> I'm
> > sure a solution can be found but I think we'd need some concerted effort
> on
> > reducing the test footprint.
> >
> > We talked before (Daniel's suggestion) about treating more of the tests
> as
> > integration tests.  This would help as much of the test time is spent
> > starting and stopping Drillbits for each test class.  If we only did this
> > once for all those tests, the footprint would be much smaller.
> >
> >
> >
> > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> > wrote:
> >
> >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
> >> wrote:
> >>
> >> > We tried Travis before.  The problem is that travis's nodes aren't
> >> > substantial enough to complete our test suite within their timeout.
> >> >
> >>
> >> Haven't the tests been substantially improved since then?
> >>
> >> Can the tests be segregated into pieces so Travis can still do some
> useful
> >> work?
> >>
> >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jim Scott <js...@maprtech.com>.
All commit information will end up on the Jira ticket if the Jira ticket
number is listed in the commit. And as long as Jira and github integration
is setup.
On Jul 8, 2015 8:20 PM, "Parth Chandra" <pa...@apache.org> wrote:

> On Wed, Jul 8, 2015 at 5:15 PM, Jason Altekruse <al...@gmail.com>
> wrote:
>
> >
> > Questions:
> > Do we need to have the patches as they change throughout a review on the
> > apache infrastructure?
> >
>
> I have gone back to a review afterwards to see whether I missed something
> even after the patch was merged. But I don't think there has ever been the
> need to review the updates to the patches.
>
>
> > If not, do we want to have final patch on the JIRA? Or should users
> > interested in backporting a bug fix just go right to the repo to find the
> > changeset to apply?
> >
> >
> The patch is not really needed in  the JIRA I think. It would be nice if
> the commit id itself is in the JIRA when the issue is closed. Going to the
> pull request to get the commit id is a few clicks too many.
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
On Mon, Jul 20, 2015 at 11:23 AM, Jason Altekruse <al...@gmail.com>
wrote:

>
> I will be opening an infra ticket if there is not already one to request
> these comments also be reflected if possible.
>
>
Good catch.

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jason Altekruse <al...@gmail.com>.
Hello All,

As you might have noticed, we have started opening up pull requests for new
patches and going through the review cycle over on github.

I wanted to bring up one small issue that Sudheesh noticed with my review
of one of his patches. He had posted his change-set in two parts (the
primary benefit of the PRs over reviewboard) I was looking at each of the
commits individually and making in-line comments. When I looked back at the
PR I saw my comments listed and assigned the JIRA back to him address my
comments. Unfortunately, it looks like comments on the individual commits
are aggregated into the pull request, but these comments have not been
reflected back on JIRA. It looks like we should be making the comments on
the combination of all of the changes available in a PR with the "files
changed" link, rather than commenting on the individual commits. We can
still use the separation of the commits to look at individual work units,
but it would be best to have the discussion reflected on JIRA.

Example of comments that appear on the PR but were not copied to JIRA:
https://github.com/apache/drill/pull/90
https://issues.apache.org/jira/browse/DRILL-2304

I will be opening an infra ticket if there is not already one to request
these comments also be reflected if possible.

On Wed, Jul 8, 2015 at 10:53 PM, Ted Dunning <te...@gmail.com> wrote:

> On Wed, Jul 8, 2015 at 6:19 PM, Parth Chandra <pa...@apache.org> wrote:
>
> > > If not, do we want to have final patch on the JIRA? Or should users
> > > interested in backporting a bug fix just go right to the repo to find
> the
> > > changeset to apply?
> > >
> > >
> > The patch is not really needed in  the JIRA I think. It would be nice if
> > the commit id itself is in the JIRA when the issue is closed. Going to
> the
> > pull request to get the commit id is a few clicks too many.
>
>
> Concur with this.
>
> Git will have all the history needed.  If the JIRA has the discussion, this
> is good.
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
On Wed, Jul 8, 2015 at 6:19 PM, Parth Chandra <pa...@apache.org> wrote:

> > If not, do we want to have final patch on the JIRA? Or should users
> > interested in backporting a bug fix just go right to the repo to find the
> > changeset to apply?
> >
> >
> The patch is not really needed in  the JIRA I think. It would be nice if
> the commit id itself is in the JIRA when the issue is closed. Going to the
> pull request to get the commit id is a few clicks too many.


Concur with this.

Git will have all the history needed.  If the JIRA has the discussion, this
is good.

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Parth Chandra <pa...@apache.org>.
On Wed, Jul 8, 2015 at 5:15 PM, Jason Altekruse <al...@gmail.com>
wrote:

>
> Questions:
> Do we need to have the patches as they change throughout a review on the
> apache infrastructure?
>

I have gone back to a review afterwards to see whether I missed something
even after the patch was merged. But I don't think there has ever been the
need to review the updates to the patches.


> If not, do we want to have final patch on the JIRA? Or should users
> interested in backporting a bug fix just go right to the repo to find the
> changeset to apply?
>
>
The patch is not really needed in  the JIRA I think. It would be nice if
the commit id itself is in the JIRA when the issue is closed. Going to the
pull request to get the commit id is a few clicks too many.

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jason Altekruse <al...@gmail.com>.
Hanifi went through a complete review on a patch with github and has
checked in his change.

To see how github reflected the discussion on JIRA you can look here:

https://issues.apache.org/jira/browse/DRILL-3441

It looks like my section of the process relating to posting patches to JIRA
as well as creating/updating your pull request might not be needed. The
content of the intermediate work while it is in review does not seem to end
up on JIRA or the mailing list with the automatic integration.

Questions:
Do we need to have the patches as they change throughout a review on the
apache infrastructure?
If not, do we want to have final patch on the JIRA? Or should users
interested in backporting a bug fix just go right to the repo to find the
changeset to apply?

On Tue, Jul 7, 2015 at 11:17 PM, Ted Dunning <te...@gmail.com> wrote:

>
> If the code patches get chexked in on the apache git, they propagate to gh
> and close the PR request there.
>
> No fancy plugins needed for that.  Mentioning the jira gets a link to the
> PR and the content of the PR gets mirrored to the mailing list as it
> happens as well.
>
> Sent from my iPhone
>
> > On Jul 7, 2015, at 17:51, Jason Altekruse <al...@gmail.com>
> wrote:
> >
> > I'm going to do a little more digging, because it is not clear to me if
> > the actual content of the patches makes it back to the JIRA with this
> > automatic integration. It would be nice to remove the extra step of
> > generating and uploading patch files, but we want to make sure we have
> the
> > code changes stored on apache infra somehow.
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
If the code patches get chexked in on the apache git, they propagate to gh and close the PR request there. 

No fancy plugins needed for that.  Mentioning the jira gets a link to the PR and the content of the PR gets mirrored to the mailing list as it happens as well. 

Sent from my iPhone

> On Jul 7, 2015, at 17:51, Jason Altekruse <al...@gmail.com> wrote:
> 
> I'm going to do a little more digging, because it is not clear to me if
> the actual content of the patches makes it back to the JIRA with this
> automatic integration. It would be nice to remove the extra step of
> generating and uploading patch files, but we want to make sure we have the
> code changes stored on apache infra somehow.

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Matthew Burgess <ma...@gmail.com>.
If this desired solution is not available, feasible, or otherwise painful, I
can follow up with our folks, possibly contributing some Python scripts to
handle these tasks, as we are also trying to tie GitHub to Jira without IT
being in the critical path (but of course informed). Personally, I've been
looking at a Continuous Delivery pipeline from GitHub to Jira/ReviewNinja
<http://www.review.ninja/>  to Travis <https://travis-ci.org/> , then Travis
to Bintray <https://bintray.com/> , etc.

From:  Jason Altekruse <al...@gmail.com>
Reply-To:  <de...@drill.apache.org>
Date:  Tuesday, July 7, 2015 at 8:51 PM
To:  <de...@drill.apache.org>
Subject:  Re: [DISCUSS] Allowing the option to use github pull requests in
place of reviewboard

We don't actually have permission to change the JIRA instance ourselves,
but we can open an apache infra ticket for it. Here is a blog post about
some integration they added fairly recently. It says here that they need to
enable several features as requested by particular projects. Is there
anything listed here we don't want turned on?

 I'm going to do a little more digging, because it is not clear to me if
the actual content of the patches makes it back to the JIRA with this
automatic integration. It would be nice to remove the extra step of
generating and uploading patch files, but we want to make sure we have the
code changes stored on apache infra somehow.

https://blogs.apache.org/infra/entry/improved_integration_between_apache_and

On Tue, Jul 7, 2015 at 5:41 PM, Jim Scott <js...@maprtech.com> wrote:

>  The main requirement is that it requires a person with a high enough
>  permission level in JIRA to setup the connector to github. They also need
>  to be able to configure the github side as well.
> 
>  On Tue, Jul 7, 2015 at 7:38 PM, Matt Burgess <ma...@gmail.com> wrote:
> 
>>  > Is it difficult to get the DVCS connector installed/enabled for Drill's
>>  > Jira? I know for my company it's been sitting on the to-do list for a
>>  > while, but we haven't actually installed it.
>>  >
>>  > Sent from my iPhone
>>  >
>>>  > > On Jul 7, 2015, at 8:25 PM, Jim Scott <js...@maprtech.com> wrote:
>>>  > >
>>>  > > The instructions for setting up JIRA to integrate with github is here:
>>>  > > http://blogs.atlassian.com/2014/04/connecting-jira-6-2-github/
>>>  > >
>>>  > > As Ted points out, in this case any comment on a commit that says
>>  > DRILL-NNN
>>>  > > would get found by JIRA and would automatically be added to the Jira
>>  > ticket
>>>  > > for the history to be reviewed.
>>>  > >
>>>>  > >> On Tue, Jul 7, 2015 at 4:32 PM, Ted Dunning <te...@gmail.com>
>>  > wrote:
>>>>  > >>
>>>>  > >> The only Apache requirement is that the mailing list + JIRA makes
>  sense
>>  > and
>>>>  > >> is readable as a history of the project.  Keeping the issues on >>>>
github
>>>>  > >> won't fly.
>>>>  > >>
>>>>  > >> It is pretty easy to get github to hook into JIRA pretty well if the
>  PR
>>>>  > >> messages have a reference to the JIRA.  I don't know the details.
>>>>  > >>
>>>>  > >> The suggestion to require the person who merges to include a
>>>> code-word
>>  > in
>>>>  > >> the message is a very good one.
>>>>  > >>
>>>>  > >>
>>>>  > >>
>>>>>  > >>> On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org>
>>  > wrote:
>>>>>  > >>>
>>>>>  > >>> I don't think we need anything formal.  Do you want to propose a
few
>>>>  > >> simple
>>>>>  > >>> guidelines?
>>>>>  > >>>
>>>>>  > >>> E.g. Should we link to PRs on JIRA, do we really need a JIRA, the
>>  > merger
>>>>  > >> is
>>>>>  > >>> responsible for including the "close #123" syntax in the commit,
etc.
>>>>>  > >>>
>>>>>  > >>> On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <
>>>>  > >> altekrusejason@gmail.com>
>>>>>  > >>> wrote:
>>>>>  > >>>
>>>>>>  > >>>> Seems to be enough consensus that this is beneficial. I took a
look
>  at
>>>>>  > >>> the
>>>>>>  > >>>> bylaws and it doesn't say anything specific there about an >>>>>>
official
>>>>>  > >>> review
>>>>>>  > >>>> process. Is there a need to start a separate vote thread before
>  making
>>>>  > >> a
>>>>>>  > >>>> change like this?
>>>>>>  > >>>>
>>>>>>  > >>>> I would be in favor of allowing both for a little bit, if it is a
>>>>>>  > >>>> significant improvement we can move to completely deprecate
>>>>  > >> reviewboard.
>>>>>>  > >>>>
>>>>>>>  > >>>>> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau
>>>>>>> <jacques@apache.org
>>  >
>>>>>>  > >>>> wrote:
>>>>>>  > >>>>
>>>>>>>  > >>>>> What did we decide here?
>>>>>>>  > >>>>>
>>>>>>>  > >>>>> Are we going to move forward with trying out pull requests?  If
so,
>>>>  > >> do
>>>>>  > >>> we
>>>>>>>  > >>>>> want to start having everyone do it or suggest only one or two
do
>  it
>>>>  > >> to
>>>>>>>  > >>>>> start?
>>>>>>>  > >>>>>
>>>>>>>  > >>>>> thoughts?
>>>>>>>  > >>>>> Jacques
>>>>>>>  > >>>>>
>>>>>>>  > >>>>> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <
>  jacques@apache.org>
>>>>>>>  > >>>>> wrote:
>>>>>>>  > >>>>>
>>>>>>>>  > >>>>>> I can't remember the rule.  I think it was 3gb and 15
>>>>>>>> minutes.  In
>>>>>>  > >>>> order
>>>>>>>>  > >>>>>> to get under 3 gb, I think we need to run single threaded.
Last I
>>>>>>>  > >>>>> checked,
>>>>>>>>  > >>>>>> running with 4 threads on dedicated hardware completes in ~12
>>>>>  > >>> minutes.
>>>>>>>>  > >>>>>> However, the Travis instances used to be really slow virtual
>>>>>  > >>> machines.
>>>>>>>  > >>>>> I'm
>>>>>>>>  > >>>>>> sure a solution can be found but I think we'd need some
concerted
>>>>>>  > >>>> effort
>>>>>>>  > >>>>> on
>>>>>>>>  > >>>>>> reducing the test footprint.
>>>>>>>>  > >>>>>>
>>>>>>>>  > >>>>>> We talked before (Daniel's suggestion) about treating more of
the
>>>>>  > >>> tests
>>>>>>>  > >>>>> as
>>>>>>>>  > >>>>>> integration tests.  This would help as much of the test time
is
>>>>  > >> spent
>>>>>>>>  > >>>>>> starting and stopping Drillbits for each test class.  If we
only
>>>>  > >> did
>>>>>>  > >>>> this
>>>>>>>>  > >>>>>> once for all those tests, the footprint would be much
smaller.
>>>>>>>>  > >>>>>>
>>>>>>>>  > >>>>>>
>>>>>>>>  > >>>>>>
>>>>>>>>  > >>>>>> On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <
>>>>  > >> ted.dunning@gmail.com>
>>>>>>>>  > >>>>>> wrote:
>>>>>>>>  > >>>>>>
>>>>>>>>>  > >>>>>>> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
>>>>>  > >>> jacques@apache.org>
>>>>>>>>>  > >>>>>>> wrote:
>>>>>>>>>  > >>>>>>>
>>>>>>>>>>  > >>>>>>>> We tried Travis before.  The problem is that travis's
nodes
>>>>  > >> aren't
>>>>>>>>>>  > >>>>>>>> substantial enough to complete our test suite within
their
>>>>>  > >>> timeout.
>>>>>>>>>  > >>>>>>>
>>>>>>>>>  > >>>>>>> Haven't the tests been substantially improved since then?
>>>>>>>>>  > >>>>>>>
>>>>>>>>>  > >>>>>>> Can the tests be segregated into pieces so Travis can still
do
>>>>  > >> some
>>>>>>>  > >>>>> useful
>>>>>>>>>  > >>>>>>> work?
>>  >
> 




Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jason Altekruse <al...@gmail.com>.
We don't actually have permission to change the JIRA instance ourselves,
but we can open an apache infra ticket for it. Here is a blog post about
some integration they added fairly recently. It says here that they need to
enable several features as requested by particular projects. Is there
anything listed here we don't want turned on?

 I'm going to do a little more digging, because it is not clear to me if
the actual content of the patches makes it back to the JIRA with this
automatic integration. It would be nice to remove the extra step of
generating and uploading patch files, but we want to make sure we have the
code changes stored on apache infra somehow.

https://blogs.apache.org/infra/entry/improved_integration_between_apache_and

On Tue, Jul 7, 2015 at 5:41 PM, Jim Scott <js...@maprtech.com> wrote:

> The main requirement is that it requires a person with a high enough
> permission level in JIRA to setup the connector to github. They also need
> to be able to configure the github side as well.
>
> On Tue, Jul 7, 2015 at 7:38 PM, Matt Burgess <ma...@gmail.com> wrote:
>
> > Is it difficult to get the DVCS connector installed/enabled for Drill's
> > Jira? I know for my company it's been sitting on the to-do list for a
> > while, but we haven't actually installed it.
> >
> > Sent from my iPhone
> >
> > > On Jul 7, 2015, at 8:25 PM, Jim Scott <js...@maprtech.com> wrote:
> > >
> > > The instructions for setting up JIRA to integrate with github is here:
> > > http://blogs.atlassian.com/2014/04/connecting-jira-6-2-github/
> > >
> > > As Ted points out, in this case any comment on a commit that says
> > DRILL-NNN
> > > would get found by JIRA and would automatically be added to the Jira
> > ticket
> > > for the history to be reviewed.
> > >
> > >> On Tue, Jul 7, 2015 at 4:32 PM, Ted Dunning <te...@gmail.com>
> > wrote:
> > >>
> > >> The only Apache requirement is that the mailing list + JIRA makes
> sense
> > and
> > >> is readable as a history of the project.  Keeping the issues on github
> > >> won't fly.
> > >>
> > >> It is pretty easy to get github to hook into JIRA pretty well if the
> PR
> > >> messages have a reference to the JIRA.  I don't know the details.
> > >>
> > >> The suggestion to require the person who merges to include a code-word
> > in
> > >> the message is a very good one.
> > >>
> > >>
> > >>
> > >>> On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> > >>>
> > >>> I don't think we need anything formal.  Do you want to propose a few
> > >> simple
> > >>> guidelines?
> > >>>
> > >>> E.g. Should we link to PRs on JIRA, do we really need a JIRA, the
> > merger
> > >> is
> > >>> responsible for including the "close #123" syntax in the commit, etc.
> > >>>
> > >>> On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <
> > >> altekrusejason@gmail.com>
> > >>> wrote:
> > >>>
> > >>>> Seems to be enough consensus that this is beneficial. I took a look
> at
> > >>> the
> > >>>> bylaws and it doesn't say anything specific there about an official
> > >>> review
> > >>>> process. Is there a need to start a separate vote thread before
> making
> > >> a
> > >>>> change like this?
> > >>>>
> > >>>> I would be in favor of allowing both for a little bit, if it is a
> > >>>> significant improvement we can move to completely deprecate
> > >> reviewboard.
> > >>>>
> > >>>>> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <jacques@apache.org
> >
> > >>>> wrote:
> > >>>>
> > >>>>> What did we decide here?
> > >>>>>
> > >>>>> Are we going to move forward with trying out pull requests?  If so,
> > >> do
> > >>> we
> > >>>>> want to start having everyone do it or suggest only one or two do
> it
> > >> to
> > >>>>> start?
> > >>>>>
> > >>>>> thoughts?
> > >>>>> Jacques
> > >>>>>
> > >>>>> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <
> jacques@apache.org>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> > >>>> order
> > >>>>>> to get under 3 gb, I think we need to run single threaded.  Last I
> > >>>>> checked,
> > >>>>>> running with 4 threads on dedicated hardware completes in ~12
> > >>> minutes.
> > >>>>>> However, the Travis instances used to be really slow virtual
> > >>> machines.
> > >>>>> I'm
> > >>>>>> sure a solution can be found but I think we'd need some concerted
> > >>>> effort
> > >>>>> on
> > >>>>>> reducing the test footprint.
> > >>>>>>
> > >>>>>> We talked before (Daniel's suggestion) about treating more of the
> > >>> tests
> > >>>>> as
> > >>>>>> integration tests.  This would help as much of the test time is
> > >> spent
> > >>>>>> starting and stopping Drillbits for each test class.  If we only
> > >> did
> > >>>> this
> > >>>>>> once for all those tests, the footprint would be much smaller.
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <
> > >> ted.dunning@gmail.com>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
> > >>> jacques@apache.org>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> We tried Travis before.  The problem is that travis's nodes
> > >> aren't
> > >>>>>>>> substantial enough to complete our test suite within their
> > >>> timeout.
> > >>>>>>>
> > >>>>>>> Haven't the tests been substantially improved since then?
> > >>>>>>>
> > >>>>>>> Can the tests be segregated into pieces so Travis can still do
> > >> some
> > >>>>> useful
> > >>>>>>> work?
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jim Scott <js...@maprtech.com>.
The main requirement is that it requires a person with a high enough
permission level in JIRA to setup the connector to github. They also need
to be able to configure the github side as well.

On Tue, Jul 7, 2015 at 7:38 PM, Matt Burgess <ma...@gmail.com> wrote:

> Is it difficult to get the DVCS connector installed/enabled for Drill's
> Jira? I know for my company it's been sitting on the to-do list for a
> while, but we haven't actually installed it.
>
> Sent from my iPhone
>
> > On Jul 7, 2015, at 8:25 PM, Jim Scott <js...@maprtech.com> wrote:
> >
> > The instructions for setting up JIRA to integrate with github is here:
> > http://blogs.atlassian.com/2014/04/connecting-jira-6-2-github/
> >
> > As Ted points out, in this case any comment on a commit that says
> DRILL-NNN
> > would get found by JIRA and would automatically be added to the Jira
> ticket
> > for the history to be reviewed.
> >
> >> On Tue, Jul 7, 2015 at 4:32 PM, Ted Dunning <te...@gmail.com>
> wrote:
> >>
> >> The only Apache requirement is that the mailing list + JIRA makes sense
> and
> >> is readable as a history of the project.  Keeping the issues on github
> >> won't fly.
> >>
> >> It is pretty easy to get github to hook into JIRA pretty well if the PR
> >> messages have a reference to the JIRA.  I don't know the details.
> >>
> >> The suggestion to require the person who merges to include a code-word
> in
> >> the message is a very good one.
> >>
> >>
> >>
> >>> On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org>
> wrote:
> >>>
> >>> I don't think we need anything formal.  Do you want to propose a few
> >> simple
> >>> guidelines?
> >>>
> >>> E.g. Should we link to PRs on JIRA, do we really need a JIRA, the
> merger
> >> is
> >>> responsible for including the "close #123" syntax in the commit, etc.
> >>>
> >>> On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <
> >> altekrusejason@gmail.com>
> >>> wrote:
> >>>
> >>>> Seems to be enough consensus that this is beneficial. I took a look at
> >>> the
> >>>> bylaws and it doesn't say anything specific there about an official
> >>> review
> >>>> process. Is there a need to start a separate vote thread before making
> >> a
> >>>> change like this?
> >>>>
> >>>> I would be in favor of allowing both for a little bit, if it is a
> >>>> significant improvement we can move to completely deprecate
> >> reviewboard.
> >>>>
> >>>>> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org>
> >>>> wrote:
> >>>>
> >>>>> What did we decide here?
> >>>>>
> >>>>> Are we going to move forward with trying out pull requests?  If so,
> >> do
> >>> we
> >>>>> want to start having everyone do it or suggest only one or two do it
> >> to
> >>>>> start?
> >>>>>
> >>>>> thoughts?
> >>>>> Jacques
> >>>>>
> >>>>> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> >>>>> wrote:
> >>>>>
> >>>>>> I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> >>>> order
> >>>>>> to get under 3 gb, I think we need to run single threaded.  Last I
> >>>>> checked,
> >>>>>> running with 4 threads on dedicated hardware completes in ~12
> >>> minutes.
> >>>>>> However, the Travis instances used to be really slow virtual
> >>> machines.
> >>>>> I'm
> >>>>>> sure a solution can be found but I think we'd need some concerted
> >>>> effort
> >>>>> on
> >>>>>> reducing the test footprint.
> >>>>>>
> >>>>>> We talked before (Daniel's suggestion) about treating more of the
> >>> tests
> >>>>> as
> >>>>>> integration tests.  This would help as much of the test time is
> >> spent
> >>>>>> starting and stopping Drillbits for each test class.  If we only
> >> did
> >>>> this
> >>>>>> once for all those tests, the footprint would be much smaller.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <
> >> ted.dunning@gmail.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
> >>> jacques@apache.org>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> We tried Travis before.  The problem is that travis's nodes
> >> aren't
> >>>>>>>> substantial enough to complete our test suite within their
> >>> timeout.
> >>>>>>>
> >>>>>>> Haven't the tests been substantially improved since then?
> >>>>>>>
> >>>>>>> Can the tests be segregated into pieces so Travis can still do
> >> some
> >>>>> useful
> >>>>>>> work?
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Matt Burgess <ma...@gmail.com>.
Is it difficult to get the DVCS connector installed/enabled for Drill's Jira? I know for my company it's been sitting on the to-do list for a while, but we haven't actually installed it.

Sent from my iPhone

> On Jul 7, 2015, at 8:25 PM, Jim Scott <js...@maprtech.com> wrote:
> 
> The instructions for setting up JIRA to integrate with github is here:
> http://blogs.atlassian.com/2014/04/connecting-jira-6-2-github/
> 
> As Ted points out, in this case any comment on a commit that says DRILL-NNN
> would get found by JIRA and would automatically be added to the Jira ticket
> for the history to be reviewed.
> 
>> On Tue, Jul 7, 2015 at 4:32 PM, Ted Dunning <te...@gmail.com> wrote:
>> 
>> The only Apache requirement is that the mailing list + JIRA makes sense and
>> is readable as a history of the project.  Keeping the issues on github
>> won't fly.
>> 
>> It is pretty easy to get github to hook into JIRA pretty well if the PR
>> messages have a reference to the JIRA.  I don't know the details.
>> 
>> The suggestion to require the person who merges to include a code-word in
>> the message is a very good one.
>> 
>> 
>> 
>>> On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org> wrote:
>>> 
>>> I don't think we need anything formal.  Do you want to propose a few
>> simple
>>> guidelines?
>>> 
>>> E.g. Should we link to PRs on JIRA, do we really need a JIRA, the merger
>> is
>>> responsible for including the "close #123" syntax in the commit, etc.
>>> 
>>> On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <
>> altekrusejason@gmail.com>
>>> wrote:
>>> 
>>>> Seems to be enough consensus that this is beneficial. I took a look at
>>> the
>>>> bylaws and it doesn't say anything specific there about an official
>>> review
>>>> process. Is there a need to start a separate vote thread before making
>> a
>>>> change like this?
>>>> 
>>>> I would be in favor of allowing both for a little bit, if it is a
>>>> significant improvement we can move to completely deprecate
>> reviewboard.
>>>> 
>>>>> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org>
>>>> wrote:
>>>> 
>>>>> What did we decide here?
>>>>> 
>>>>> Are we going to move forward with trying out pull requests?  If so,
>> do
>>> we
>>>>> want to start having everyone do it or suggest only one or two do it
>> to
>>>>> start?
>>>>> 
>>>>> thoughts?
>>>>> Jacques
>>>>> 
>>>>> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
>>>>> wrote:
>>>>> 
>>>>>> I can't remember the rule.  I think it was 3gb and 15 minutes.  In
>>>> order
>>>>>> to get under 3 gb, I think we need to run single threaded.  Last I
>>>>> checked,
>>>>>> running with 4 threads on dedicated hardware completes in ~12
>>> minutes.
>>>>>> However, the Travis instances used to be really slow virtual
>>> machines.
>>>>> I'm
>>>>>> sure a solution can be found but I think we'd need some concerted
>>>> effort
>>>>> on
>>>>>> reducing the test footprint.
>>>>>> 
>>>>>> We talked before (Daniel's suggestion) about treating more of the
>>> tests
>>>>> as
>>>>>> integration tests.  This would help as much of the test time is
>> spent
>>>>>> starting and stopping Drillbits for each test class.  If we only
>> did
>>>> this
>>>>>> once for all those tests, the footprint would be much smaller.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <
>> ted.dunning@gmail.com>
>>>>>> wrote:
>>>>>> 
>>>>>>> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
>>> jacques@apache.org>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> We tried Travis before.  The problem is that travis's nodes
>> aren't
>>>>>>>> substantial enough to complete our test suite within their
>>> timeout.
>>>>>>> 
>>>>>>> Haven't the tests been substantially improved since then?
>>>>>>> 
>>>>>>> Can the tests be segregated into pieces so Travis can still do
>> some
>>>>> useful
>>>>>>> work?
> 
> 
> 
> -- 
> *Jim Scott*
> Director, Enterprise Strategy & Architecture
> +1 (347) 746-9281
> 
> <http://www.mapr.com/>
> [image: MapR Technologies] <http://www.mapr.com>
> 
> Now Available - Free Hadoop On-Demand Training
> <http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jim Scott <js...@maprtech.com>.
The instructions for setting up JIRA to integrate with github is here:
http://blogs.atlassian.com/2014/04/connecting-jira-6-2-github/

As Ted points out, in this case any comment on a commit that says DRILL-NNN
would get found by JIRA and would automatically be added to the Jira ticket
for the history to be reviewed.

On Tue, Jul 7, 2015 at 4:32 PM, Ted Dunning <te...@gmail.com> wrote:

> The only Apache requirement is that the mailing list + JIRA makes sense and
> is readable as a history of the project.  Keeping the issues on github
> won't fly.
>
> It is pretty easy to get github to hook into JIRA pretty well if the PR
> messages have a reference to the JIRA.  I don't know the details.
>
> The suggestion to require the person who merges to include a code-word in
> the message is a very good one.
>
>
>
> On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org> wrote:
>
> > I don't think we need anything formal.  Do you want to propose a few
> simple
> > guidelines?
> >
> > E.g. Should we link to PRs on JIRA, do we really need a JIRA, the merger
> is
> > responsible for including the "close #123" syntax in the commit, etc.
> >
> > On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <
> altekrusejason@gmail.com>
> > wrote:
> >
> > > Seems to be enough consensus that this is beneficial. I took a look at
> > the
> > > bylaws and it doesn't say anything specific there about an official
> > review
> > > process. Is there a need to start a separate vote thread before making
> a
> > > change like this?
> > >
> > > I would be in favor of allowing both for a little bit, if it is a
> > > significant improvement we can move to completely deprecate
> reviewboard.
> > >
> > > On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> > >
> > > > What did we decide here?
> > > >
> > > > Are we going to move forward with trying out pull requests?  If so,
> do
> > we
> > > > want to start having everyone do it or suggest only one or two do it
> to
> > > > start?
> > > >
> > > > thoughts?
> > > > Jacques
> > > >
> > > > On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> > > > wrote:
> > > >
> > > > > I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> > > order
> > > > > to get under 3 gb, I think we need to run single threaded.  Last I
> > > > checked,
> > > > > running with 4 threads on dedicated hardware completes in ~12
> > minutes.
> > > > > However, the Travis instances used to be really slow virtual
> > machines.
> > > > I'm
> > > > > sure a solution can be found but I think we'd need some concerted
> > > effort
> > > > on
> > > > > reducing the test footprint.
> > > > >
> > > > > We talked before (Daniel's suggestion) about treating more of the
> > tests
> > > > as
> > > > > integration tests.  This would help as much of the test time is
> spent
> > > > > starting and stopping Drillbits for each test class.  If we only
> did
> > > this
> > > > > once for all those tests, the footprint would be much smaller.
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <
> ted.dunning@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
> > jacques@apache.org>
> > > > >> wrote:
> > > > >>
> > > > >> > We tried Travis before.  The problem is that travis's nodes
> aren't
> > > > >> > substantial enough to complete our test suite within their
> > timeout.
> > > > >> >
> > > > >>
> > > > >> Haven't the tests been substantially improved since then?
> > > > >>
> > > > >> Can the tests be segregated into pieces so Travis can still do
> some
> > > > useful
> > > > >> work?
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>



-- 
*Jim Scott*
Director, Enterprise Strategy & Architecture
+1 (347) 746-9281

 <http://www.mapr.com/>
[image: MapR Technologies] <http://www.mapr.com>

Now Available - Free Hadoop On-Demand Training
<http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
The only Apache requirement is that the mailing list + JIRA makes sense and
is readable as a history of the project.  Keeping the issues on github
won't fly.

It is pretty easy to get github to hook into JIRA pretty well if the PR
messages have a reference to the JIRA.  I don't know the details.

The suggestion to require the person who merges to include a code-word in
the message is a very good one.



On Tue, Jul 7, 2015 at 1:35 PM, Jacques Nadeau <ja...@apache.org> wrote:

> I don't think we need anything formal.  Do you want to propose a few simple
> guidelines?
>
> E.g. Should we link to PRs on JIRA, do we really need a JIRA, the merger is
> responsible for including the "close #123" syntax in the commit, etc.
>
> On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <al...@gmail.com>
> wrote:
>
> > Seems to be enough consensus that this is beneficial. I took a look at
> the
> > bylaws and it doesn't say anything specific there about an official
> review
> > process. Is there a need to start a separate vote thread before making a
> > change like this?
> >
> > I would be in favor of allowing both for a little bit, if it is a
> > significant improvement we can move to completely deprecate reviewboard.
> >
> > On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org>
> wrote:
> >
> > > What did we decide here?
> > >
> > > Are we going to move forward with trying out pull requests?  If so, do
> we
> > > want to start having everyone do it or suggest only one or two do it to
> > > start?
> > >
> > > thoughts?
> > > Jacques
> > >
> > > On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> > > wrote:
> > >
> > > > I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> > order
> > > > to get under 3 gb, I think we need to run single threaded.  Last I
> > > checked,
> > > > running with 4 threads on dedicated hardware completes in ~12
> minutes.
> > > > However, the Travis instances used to be really slow virtual
> machines.
> > > I'm
> > > > sure a solution can be found but I think we'd need some concerted
> > effort
> > > on
> > > > reducing the test footprint.
> > > >
> > > > We talked before (Daniel's suggestion) about treating more of the
> tests
> > > as
> > > > integration tests.  This would help as much of the test time is spent
> > > > starting and stopping Drillbits for each test class.  If we only did
> > this
> > > > once for all those tests, the footprint would be much smaller.
> > > >
> > > >
> > > >
> > > > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> > > > wrote:
> > > >
> > > >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <
> jacques@apache.org>
> > > >> wrote:
> > > >>
> > > >> > We tried Travis before.  The problem is that travis's nodes aren't
> > > >> > substantial enough to complete our test suite within their
> timeout.
> > > >> >
> > > >>
> > > >> Haven't the tests been substantially improved since then?
> > > >>
> > > >> Can the tests be segregated into pieces so Travis can still do some
> > > useful
> > > >> work?
> > > >>
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jacques Nadeau <ja...@apache.org>.
I don't think we need anything formal.  Do you want to propose a few simple
guidelines?

E.g. Should we link to PRs on JIRA, do we really need a JIRA, the merger is
responsible for including the "close #123" syntax in the commit, etc.

On Tue, Jul 7, 2015 at 1:33 PM, Jason Altekruse <al...@gmail.com>
wrote:

> Seems to be enough consensus that this is beneficial. I took a look at the
> bylaws and it doesn't say anything specific there about an official review
> process. Is there a need to start a separate vote thread before making a
> change like this?
>
> I would be in favor of allowing both for a little bit, if it is a
> significant improvement we can move to completely deprecate reviewboard.
>
> On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org> wrote:
>
> > What did we decide here?
> >
> > Are we going to move forward with trying out pull requests?  If so, do we
> > want to start having everyone do it or suggest only one or two do it to
> > start?
> >
> > thoughts?
> > Jacques
> >
> > On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> >
> > > I can't remember the rule.  I think it was 3gb and 15 minutes.  In
> order
> > > to get under 3 gb, I think we need to run single threaded.  Last I
> > checked,
> > > running with 4 threads on dedicated hardware completes in ~12 minutes.
> > > However, the Travis instances used to be really slow virtual machines.
> > I'm
> > > sure a solution can be found but I think we'd need some concerted
> effort
> > on
> > > reducing the test footprint.
> > >
> > > We talked before (Daniel's suggestion) about treating more of the tests
> > as
> > > integration tests.  This would help as much of the test time is spent
> > > starting and stopping Drillbits for each test class.  If we only did
> this
> > > once for all those tests, the footprint would be much smaller.
> > >
> > >
> > >
> > > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> > > wrote:
> > >
> > >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
> > >> wrote:
> > >>
> > >> > We tried Travis before.  The problem is that travis's nodes aren't
> > >> > substantial enough to complete our test suite within their timeout.
> > >> >
> > >>
> > >> Haven't the tests been substantially improved since then?
> > >>
> > >> Can the tests be segregated into pieces so Travis can still do some
> > useful
> > >> work?
> > >>
> > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jason Altekruse <al...@gmail.com>.
Seems to be enough consensus that this is beneficial. I took a look at the
bylaws and it doesn't say anything specific there about an official review
process. Is there a need to start a separate vote thread before making a
change like this?

I would be in favor of allowing both for a little bit, if it is a
significant improvement we can move to completely deprecate reviewboard.

On Tue, Jul 7, 2015 at 1:14 PM, Jacques Nadeau <ja...@apache.org> wrote:

> What did we decide here?
>
> Are we going to move forward with trying out pull requests?  If so, do we
> want to start having everyone do it or suggest only one or two do it to
> start?
>
> thoughts?
> Jacques
>
> On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
>
> > I can't remember the rule.  I think it was 3gb and 15 minutes.  In order
> > to get under 3 gb, I think we need to run single threaded.  Last I
> checked,
> > running with 4 threads on dedicated hardware completes in ~12 minutes.
> > However, the Travis instances used to be really slow virtual machines.
> I'm
> > sure a solution can be found but I think we'd need some concerted effort
> on
> > reducing the test footprint.
> >
> > We talked before (Daniel's suggestion) about treating more of the tests
> as
> > integration tests.  This would help as much of the test time is spent
> > starting and stopping Drillbits for each test class.  If we only did this
> > once for all those tests, the footprint would be much smaller.
> >
> >
> >
> > On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> > wrote:
> >
> >> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
> >> wrote:
> >>
> >> > We tried Travis before.  The problem is that travis's nodes aren't
> >> > substantial enough to complete our test suite within their timeout.
> >> >
> >>
> >> Haven't the tests been substantially improved since then?
> >>
> >> Can the tests be segregated into pieces so Travis can still do some
> useful
> >> work?
> >>
> >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jacques Nadeau <ja...@apache.org>.
What did we decide here?

Are we going to move forward with trying out pull requests?  If so, do we
want to start having everyone do it or suggest only one or two do it to
start?

thoughts?
Jacques

On Wed, Jun 24, 2015 at 8:46 AM, Jacques Nadeau <ja...@apache.org> wrote:

> I can't remember the rule.  I think it was 3gb and 15 minutes.  In order
> to get under 3 gb, I think we need to run single threaded.  Last I checked,
> running with 4 threads on dedicated hardware completes in ~12 minutes.
> However, the Travis instances used to be really slow virtual machines.  I'm
> sure a solution can be found but I think we'd need some concerted effort on
> reducing the test footprint.
>
> We talked before (Daniel's suggestion) about treating more of the tests as
> integration tests.  This would help as much of the test time is spent
> starting and stopping Drillbits for each test class.  If we only did this
> once for all those tests, the footprint would be much smaller.
>
>
>
> On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com>
> wrote:
>
>> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
>> wrote:
>>
>> > We tried Travis before.  The problem is that travis's nodes aren't
>> > substantial enough to complete our test suite within their timeout.
>> >
>>
>> Haven't the tests been substantially improved since then?
>>
>> Can the tests be segregated into pieces so Travis can still do some useful
>> work?
>>
>
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jacques Nadeau <ja...@apache.org>.
I can't remember the rule.  I think it was 3gb and 15 minutes.  In order to
get under 3 gb, I think we need to run single threaded.  Last I checked,
running with 4 threads on dedicated hardware completes in ~12 minutes.
However, the Travis instances used to be really slow virtual machines.  I'm
sure a solution can be found but I think we'd need some concerted effort on
reducing the test footprint.

We talked before (Daniel's suggestion) about treating more of the tests as
integration tests.  This would help as much of the test time is spent
starting and stopping Drillbits for each test class.  If we only did this
once for all those tests, the footprint would be much smaller.



On Wed, Jun 24, 2015 at 8:37 AM, Ted Dunning <te...@gmail.com> wrote:

> On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
>
> > We tried Travis before.  The problem is that travis's nodes aren't
> > substantial enough to complete our test suite within their timeout.
> >
>
> Haven't the tests been substantially improved since then?
>
> Can the tests be segregated into pieces so Travis can still do some useful
> work?
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
On Wed, Jun 24, 2015 at 11:35 AM, Jacques Nadeau <ja...@apache.org> wrote:

> We tried Travis before.  The problem is that travis's nodes aren't
> substantial enough to complete our test suite within their timeout.
>

Haven't the tests been substantially improved since then?

Can the tests be segregated into pieces so Travis can still do some useful
work?

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jacques Nadeau <ja...@apache.org>.
We tried Travis before.  The problem is that travis's nodes aren't
substantial enough to complete our test suite within their timeout.

On Tue, Jun 23, 2015 at 9:32 PM, Ted Dunning <te...@gmail.com> wrote:

> Travis won't likely be faster for the CI part, but it is super easy to set
> up so it can't hurt to try it out.
>
> On Tue, Jun 23, 2015 at 9:10 PM, Chris Westin <ch...@gmail.com>
> wrote:
>
> > And I'll bet GitHub is a lot faster too.
> >
> > On Tue, Jun 23, 2015 at 2:23 PM, Hanifi Gunes <hg...@maprtech.com>
> wrote:
> >
> > > +1
> > >
> > > At the very least GitHub will be UP.
> > >
> > > On Tue, Jun 23, 2015 at 2:18 PM, Parth Chandra <pc...@maprtech.com>
> > > wrote:
> > >
> > > > +1 on trying this. RB has been pretty painful to us.
> > > >
> > > >
> > > >
> > > > On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <
> mattyb149@gmail.com>
> > > > wrote:
> > > >
> > > > > Is Travis <https://travis-ci.org/>  a viable option for the GitHub
> > > > route?
> > > > > I
> > > > > use it for my own projects to build pull requests (with additional
> > code
> > > > > quality targets like CheckStyle, PMD, etc.). Perhaps that would
> take
> > > some
> > > > > of
> > > > > the burden off the reviewers and let them focus on the proposed
> > > > > implementations, rather than some of the more tedious aspects of
> each
> > > > > review.
> > > > >
> > > > > From:  Jacques Nadeau <ja...@apache.org>
> > > > > Reply-To:  <de...@drill.apache.org>
> > > > > Date:  Monday, June 22, 2015 at 10:22 PM
> > > > > To:  "dev@drill.apache.org" <de...@drill.apache.org>
> > > > > Subject:  Re: [DISCUSS] Allowing the option to use github pull
> > requests
> > > > in
> > > > > place of reviewboard
> > > > >
> > > > > I'm up for this if we deprecate the old way.  Having two different
> > > > > processes seems like overkill.  In general, I find the review
> > interface
> > > > of
> > > > > GitHub less expressive/clear but everything else is way better.
> > > > >
> > > > > On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <
> > > sphillips@maprtech.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > >  +1
> > > > > >
> > > > > >  I am in favor of giving this a try.
> > > > > >
> > > > > >  If I remember correctly, the reason we abandoned pull requests
> > > > > originally
> > > > > >  was because we couldn't close the pull requests through Github.
> A
> > > > > solution
> > > > > >  could be for whoever pushes the commit to the apache git repo to
> > add
> > > > the
> > > > > >  Line "Closes <request number>". Github would then automatically
> > > close
> > > > > the
> > > > > >  pull request.
> > > > > >
> > > > > >  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <
> > > > > altekrusejason@gmail.com
> > > > > >>  >
> > > > > >  wrote:
> > > > > >
> > > > > >>  > Hello Drill developers,
> > > > > >>  >
> > > > > >>  > I am writing this message today to propose allowing the use
> of
> > > > github
> > > > > >  pull
> > > > > >>  > requests to perform reviews in place of the apache
> reviewboard
> > > > > instance.
> > > > > >>  >
> > > > > >>  > Reviewboard has caused a number of headaches in the past few
> > > > months,
> > > > > and
> > > > > >  I
> > > > > >>  > think its time to evaluate the benefits of the apache
> > > > infrastructure
> > > > > >>  > relative to the actual cost of using it in practice.
> > > > > >>  >
> > > > > >>  > For clarity of the discussion, we cannot use the complete
> > github
> > > > > >  workflow.
> > > > > >>  > Comitters will still need to use patch files, or check out
> the
> > > > branch
> > > > > >  used
> > > > > >>  > in the review request and push to apache master manually. I
> am
> > > not
> > > > > >>  > advocating for using a merging strategy with git, just for
> > using
> > > > the
> > > > > >  github
> > > > > >>  > web UI for reviews. I expect anyone generating a chain of
> > commits
> > > > as
> > > > > >>  > described below to use the rebasing workflow we do today.
> > > > > Additionally
> > > > > >  devs
> > > > > >>  > should only be breaking up work to make it easier to review,
> we
> > > > will
> > > > > not
> > > > > >  be
> > > > > >>  > reviewing branches that contain a bunch of useless WIP
> commits.
> > > > > >>  >
> > > > > >>  > A few examples of problems I have experienced with
> reviewboard
> > > > > include:
> > > > > >>  > corruption of patches when they are downloaded, the web
> > interface
> > > > > showing
> > > > > >>  > inconsistent content from the raw diff, and random rejection
> of
> > > > > patches
> > > > > >>  > that are based directly on the head of apache master.
> > > > > >>  >
> > > > > >>  > These are all serious blockers for getting code reviewed and
> > > > > integrated
> > > > > >>  > into the master branch in a timely manner.
> > > > > >>  >
> > > > > >>  > In addition to serious bugs in reviewboard, there are a
> number
> > of
> > > > > >>  > difficulties with the combination of our typical dev workflow
> > and
> > > > how
> > > > > >>  > reviewboard works with patches. As we are still adding
> features
> > > to
> > > > > Drill,
> > > > > >>  > we often have several weeks of work to submit in response to
> a
> > > JIRA
> > > > > or
> > > > > >>  > series of related JIRAs. Sometimes this work can be broken up
> > > into
> > > > > >>  > independent reviewable units, and other times it cannot.
> When a
> > > > > series of
> > > > > >>  > changes requires a mixture of refactoring and additions, the
> > > > process
> > > > > is
> > > > > >>  > currently quite painful. Ether reviewers need to look
> through a
> > > > giant
> > > > > >  messy
> > > > > >>  > diff, or the submitters need to do a lot of extra work. This
> > > > > involves not
> > > > > >>  > only organizing their work into a reviewable series of
> commits,
> > > but
> > > > > also
> > > > > >>  > generating redundant squashed versions of the intermediate
> work
> > > to
> > > > > make
> > > > > >>  > reviewboard happy.
> > > > > >>  >
> > > > > >>  > For a relatively simple 3 part change, this involves
> creating 3
> > > > > >  reviewboard
> > > > > >>  > pages. The first will contain the first commit by itself. The
> > > > second
> > > > > will
> > > > > >>  > have the first commits patch as a parent patch with the next
> > > change
> > > > > in
> > > > > >  the
> > > > > >>  > series uploaded as the core change to review. For the third
> > > > change, a
> > > > > >>  > squashed version of the first two commits must be generated
> to
> > > > serve
> > > > > as a
> > > > > >>  > parent patch and then the third changeset uploaded as the
> > > > reviewable
> > > > > >>  > change. Frequently a change to the first commit requires
> > > > > regenerating all
> > > > > >>  > of these patches and uploading them to the individual review
> > > pages.
> > > > > >>  >
> > > > > >>  > This gets even worse with larger chains of commits.
> > > > > >>  >
> > > > > >>  > It would be great if all of our changes could be small units
> of
> > > > > work, but
> > > > > >>  > very frequently we want to make sure we are ready to merge a
> > > > complete
> > > > > >>  > feature before starting the review process. We need to have a
> > > > better
> > > > > way
> > > > > >  to
> > > > > >>  > manage these large review units, as I do not see the
> > possibility
> > > of
> > > > > >>  > breaking up the work into smaller units as a likely solution.
> > We
> > > > > still
> > > > > >  have
> > > > > >>  > lots of features and system cleanup to work on.
> > > > > >>  >
> > > > > >>  > For anyone unfamiliar, github pull requests are based on a
> > branch
> > > > you
> > > > > >  push
> > > > > >>  > to your personal fork. They give space for a general
> > discussion,
> > > as
> > > > > well
> > > > > >  as
> > > > > >>  > allow commenting inline on the diff. They give a clear
> > reference
> > > to
> > > > > each
> > > > > >>  > commit in the branch, allowing reviewers to see each piece of
> > > work
> > > > > >>  > individually as well as provide a "squashed" view to see the
> > > > overall
> > > > > >>  > differences.
> > > > > >>  >
> > > > > >>  > For the sake of keeping the project history connected to
> JIRA,
> > we
> > > > > can see
> > > > > >>  > if there is enough automatic github integration or possibly
> > > upload
> > > > > patch
> > > > > >>  > files to JIRA each time we update a pull request. As an side
> > > note,
> > > > > if we
> > > > > >>  > don't need individual patches for reviewboard we could just
> put
> > > > patch
> > > > > >  files
> > > > > >>  > on JIRA that contain several commits. These are much easier
> to
> > > > > generate
> > > > > >  an
> > > > > >>  > apply than a bunch of individual files for each change. This
> > > should
> > > > > >  prevent
> > > > > >>  > JIRAs needing long lists of patches with names like
> > > > > >>  > DRILL-3000-part1-version3.patch
> > > > > >>  >
> > > > > >
> > > > > >
> > > > > >
> > > > > >  --
> > > > > >   Steven Phillips
> > > > > >   Software Engineer
> > > > > >
> > > > > >   mapr.com
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
Travis won't likely be faster for the CI part, but it is super easy to set
up so it can't hurt to try it out.

On Tue, Jun 23, 2015 at 9:10 PM, Chris Westin <ch...@gmail.com>
wrote:

> And I'll bet GitHub is a lot faster too.
>
> On Tue, Jun 23, 2015 at 2:23 PM, Hanifi Gunes <hg...@maprtech.com> wrote:
>
> > +1
> >
> > At the very least GitHub will be UP.
> >
> > On Tue, Jun 23, 2015 at 2:18 PM, Parth Chandra <pc...@maprtech.com>
> > wrote:
> >
> > > +1 on trying this. RB has been pretty painful to us.
> > >
> > >
> > >
> > > On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <ma...@gmail.com>
> > > wrote:
> > >
> > > > Is Travis <https://travis-ci.org/>  a viable option for the GitHub
> > > route?
> > > > I
> > > > use it for my own projects to build pull requests (with additional
> code
> > > > quality targets like CheckStyle, PMD, etc.). Perhaps that would take
> > some
> > > > of
> > > > the burden off the reviewers and let them focus on the proposed
> > > > implementations, rather than some of the more tedious aspects of each
> > > > review.
> > > >
> > > > From:  Jacques Nadeau <ja...@apache.org>
> > > > Reply-To:  <de...@drill.apache.org>
> > > > Date:  Monday, June 22, 2015 at 10:22 PM
> > > > To:  "dev@drill.apache.org" <de...@drill.apache.org>
> > > > Subject:  Re: [DISCUSS] Allowing the option to use github pull
> requests
> > > in
> > > > place of reviewboard
> > > >
> > > > I'm up for this if we deprecate the old way.  Having two different
> > > > processes seems like overkill.  In general, I find the review
> interface
> > > of
> > > > GitHub less expressive/clear but everything else is way better.
> > > >
> > > > On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <
> > sphillips@maprtech.com
> > > >
> > > > wrote:
> > > >
> > > > >  +1
> > > > >
> > > > >  I am in favor of giving this a try.
> > > > >
> > > > >  If I remember correctly, the reason we abandoned pull requests
> > > > originally
> > > > >  was because we couldn't close the pull requests through Github. A
> > > > solution
> > > > >  could be for whoever pushes the commit to the apache git repo to
> add
> > > the
> > > > >  Line "Closes <request number>". Github would then automatically
> > close
> > > > the
> > > > >  pull request.
> > > > >
> > > > >  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <
> > > > altekrusejason@gmail.com
> > > > >>  >
> > > > >  wrote:
> > > > >
> > > > >>  > Hello Drill developers,
> > > > >>  >
> > > > >>  > I am writing this message today to propose allowing the use of
> > > github
> > > > >  pull
> > > > >>  > requests to perform reviews in place of the apache reviewboard
> > > > instance.
> > > > >>  >
> > > > >>  > Reviewboard has caused a number of headaches in the past few
> > > months,
> > > > and
> > > > >  I
> > > > >>  > think its time to evaluate the benefits of the apache
> > > infrastructure
> > > > >>  > relative to the actual cost of using it in practice.
> > > > >>  >
> > > > >>  > For clarity of the discussion, we cannot use the complete
> github
> > > > >  workflow.
> > > > >>  > Comitters will still need to use patch files, or check out the
> > > branch
> > > > >  used
> > > > >>  > in the review request and push to apache master manually. I am
> > not
> > > > >>  > advocating for using a merging strategy with git, just for
> using
> > > the
> > > > >  github
> > > > >>  > web UI for reviews. I expect anyone generating a chain of
> commits
> > > as
> > > > >>  > described below to use the rebasing workflow we do today.
> > > > Additionally
> > > > >  devs
> > > > >>  > should only be breaking up work to make it easier to review, we
> > > will
> > > > not
> > > > >  be
> > > > >>  > reviewing branches that contain a bunch of useless WIP commits.
> > > > >>  >
> > > > >>  > A few examples of problems I have experienced with reviewboard
> > > > include:
> > > > >>  > corruption of patches when they are downloaded, the web
> interface
> > > > showing
> > > > >>  > inconsistent content from the raw diff, and random rejection of
> > > > patches
> > > > >>  > that are based directly on the head of apache master.
> > > > >>  >
> > > > >>  > These are all serious blockers for getting code reviewed and
> > > > integrated
> > > > >>  > into the master branch in a timely manner.
> > > > >>  >
> > > > >>  > In addition to serious bugs in reviewboard, there are a number
> of
> > > > >>  > difficulties with the combination of our typical dev workflow
> and
> > > how
> > > > >>  > reviewboard works with patches. As we are still adding features
> > to
> > > > Drill,
> > > > >>  > we often have several weeks of work to submit in response to a
> > JIRA
> > > > or
> > > > >>  > series of related JIRAs. Sometimes this work can be broken up
> > into
> > > > >>  > independent reviewable units, and other times it cannot. When a
> > > > series of
> > > > >>  > changes requires a mixture of refactoring and additions, the
> > > process
> > > > is
> > > > >>  > currently quite painful. Ether reviewers need to look through a
> > > giant
> > > > >  messy
> > > > >>  > diff, or the submitters need to do a lot of extra work. This
> > > > involves not
> > > > >>  > only organizing their work into a reviewable series of commits,
> > but
> > > > also
> > > > >>  > generating redundant squashed versions of the intermediate work
> > to
> > > > make
> > > > >>  > reviewboard happy.
> > > > >>  >
> > > > >>  > For a relatively simple 3 part change, this involves creating 3
> > > > >  reviewboard
> > > > >>  > pages. The first will contain the first commit by itself. The
> > > second
> > > > will
> > > > >>  > have the first commits patch as a parent patch with the next
> > change
> > > > in
> > > > >  the
> > > > >>  > series uploaded as the core change to review. For the third
> > > change, a
> > > > >>  > squashed version of the first two commits must be generated to
> > > serve
> > > > as a
> > > > >>  > parent patch and then the third changeset uploaded as the
> > > reviewable
> > > > >>  > change. Frequently a change to the first commit requires
> > > > regenerating all
> > > > >>  > of these patches and uploading them to the individual review
> > pages.
> > > > >>  >
> > > > >>  > This gets even worse with larger chains of commits.
> > > > >>  >
> > > > >>  > It would be great if all of our changes could be small units of
> > > > work, but
> > > > >>  > very frequently we want to make sure we are ready to merge a
> > > complete
> > > > >>  > feature before starting the review process. We need to have a
> > > better
> > > > way
> > > > >  to
> > > > >>  > manage these large review units, as I do not see the
> possibility
> > of
> > > > >>  > breaking up the work into smaller units as a likely solution.
> We
> > > > still
> > > > >  have
> > > > >>  > lots of features and system cleanup to work on.
> > > > >>  >
> > > > >>  > For anyone unfamiliar, github pull requests are based on a
> branch
> > > you
> > > > >  push
> > > > >>  > to your personal fork. They give space for a general
> discussion,
> > as
> > > > well
> > > > >  as
> > > > >>  > allow commenting inline on the diff. They give a clear
> reference
> > to
> > > > each
> > > > >>  > commit in the branch, allowing reviewers to see each piece of
> > work
> > > > >>  > individually as well as provide a "squashed" view to see the
> > > overall
> > > > >>  > differences.
> > > > >>  >
> > > > >>  > For the sake of keeping the project history connected to JIRA,
> we
> > > > can see
> > > > >>  > if there is enough automatic github integration or possibly
> > upload
> > > > patch
> > > > >>  > files to JIRA each time we update a pull request. As an side
> > note,
> > > > if we
> > > > >>  > don't need individual patches for reviewboard we could just put
> > > patch
> > > > >  files
> > > > >>  > on JIRA that contain several commits. These are much easier to
> > > > generate
> > > > >  an
> > > > >>  > apply than a bunch of individual files for each change. This
> > should
> > > > >  prevent
> > > > >>  > JIRAs needing long lists of patches with names like
> > > > >>  > DRILL-3000-part1-version3.patch
> > > > >>  >
> > > > >
> > > > >
> > > > >
> > > > >  --
> > > > >   Steven Phillips
> > > > >   Software Engineer
> > > > >
> > > > >   mapr.com
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Chris Westin <ch...@gmail.com>.
And I'll bet GitHub is a lot faster too.

On Tue, Jun 23, 2015 at 2:23 PM, Hanifi Gunes <hg...@maprtech.com> wrote:

> +1
>
> At the very least GitHub will be UP.
>
> On Tue, Jun 23, 2015 at 2:18 PM, Parth Chandra <pc...@maprtech.com>
> wrote:
>
> > +1 on trying this. RB has been pretty painful to us.
> >
> >
> >
> > On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <ma...@gmail.com>
> > wrote:
> >
> > > Is Travis <https://travis-ci.org/>  a viable option for the GitHub
> > route?
> > > I
> > > use it for my own projects to build pull requests (with additional code
> > > quality targets like CheckStyle, PMD, etc.). Perhaps that would take
> some
> > > of
> > > the burden off the reviewers and let them focus on the proposed
> > > implementations, rather than some of the more tedious aspects of each
> > > review.
> > >
> > > From:  Jacques Nadeau <ja...@apache.org>
> > > Reply-To:  <de...@drill.apache.org>
> > > Date:  Monday, June 22, 2015 at 10:22 PM
> > > To:  "dev@drill.apache.org" <de...@drill.apache.org>
> > > Subject:  Re: [DISCUSS] Allowing the option to use github pull requests
> > in
> > > place of reviewboard
> > >
> > > I'm up for this if we deprecate the old way.  Having two different
> > > processes seems like overkill.  In general, I find the review interface
> > of
> > > GitHub less expressive/clear but everything else is way better.
> > >
> > > On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <
> sphillips@maprtech.com
> > >
> > > wrote:
> > >
> > > >  +1
> > > >
> > > >  I am in favor of giving this a try.
> > > >
> > > >  If I remember correctly, the reason we abandoned pull requests
> > > originally
> > > >  was because we couldn't close the pull requests through Github. A
> > > solution
> > > >  could be for whoever pushes the commit to the apache git repo to add
> > the
> > > >  Line "Closes <request number>". Github would then automatically
> close
> > > the
> > > >  pull request.
> > > >
> > > >  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <
> > > altekrusejason@gmail.com
> > > >>  >
> > > >  wrote:
> > > >
> > > >>  > Hello Drill developers,
> > > >>  >
> > > >>  > I am writing this message today to propose allowing the use of
> > github
> > > >  pull
> > > >>  > requests to perform reviews in place of the apache reviewboard
> > > instance.
> > > >>  >
> > > >>  > Reviewboard has caused a number of headaches in the past few
> > months,
> > > and
> > > >  I
> > > >>  > think its time to evaluate the benefits of the apache
> > infrastructure
> > > >>  > relative to the actual cost of using it in practice.
> > > >>  >
> > > >>  > For clarity of the discussion, we cannot use the complete github
> > > >  workflow.
> > > >>  > Comitters will still need to use patch files, or check out the
> > branch
> > > >  used
> > > >>  > in the review request and push to apache master manually. I am
> not
> > > >>  > advocating for using a merging strategy with git, just for using
> > the
> > > >  github
> > > >>  > web UI for reviews. I expect anyone generating a chain of commits
> > as
> > > >>  > described below to use the rebasing workflow we do today.
> > > Additionally
> > > >  devs
> > > >>  > should only be breaking up work to make it easier to review, we
> > will
> > > not
> > > >  be
> > > >>  > reviewing branches that contain a bunch of useless WIP commits.
> > > >>  >
> > > >>  > A few examples of problems I have experienced with reviewboard
> > > include:
> > > >>  > corruption of patches when they are downloaded, the web interface
> > > showing
> > > >>  > inconsistent content from the raw diff, and random rejection of
> > > patches
> > > >>  > that are based directly on the head of apache master.
> > > >>  >
> > > >>  > These are all serious blockers for getting code reviewed and
> > > integrated
> > > >>  > into the master branch in a timely manner.
> > > >>  >
> > > >>  > In addition to serious bugs in reviewboard, there are a number of
> > > >>  > difficulties with the combination of our typical dev workflow and
> > how
> > > >>  > reviewboard works with patches. As we are still adding features
> to
> > > Drill,
> > > >>  > we often have several weeks of work to submit in response to a
> JIRA
> > > or
> > > >>  > series of related JIRAs. Sometimes this work can be broken up
> into
> > > >>  > independent reviewable units, and other times it cannot. When a
> > > series of
> > > >>  > changes requires a mixture of refactoring and additions, the
> > process
> > > is
> > > >>  > currently quite painful. Ether reviewers need to look through a
> > giant
> > > >  messy
> > > >>  > diff, or the submitters need to do a lot of extra work. This
> > > involves not
> > > >>  > only organizing their work into a reviewable series of commits,
> but
> > > also
> > > >>  > generating redundant squashed versions of the intermediate work
> to
> > > make
> > > >>  > reviewboard happy.
> > > >>  >
> > > >>  > For a relatively simple 3 part change, this involves creating 3
> > > >  reviewboard
> > > >>  > pages. The first will contain the first commit by itself. The
> > second
> > > will
> > > >>  > have the first commits patch as a parent patch with the next
> change
> > > in
> > > >  the
> > > >>  > series uploaded as the core change to review. For the third
> > change, a
> > > >>  > squashed version of the first two commits must be generated to
> > serve
> > > as a
> > > >>  > parent patch and then the third changeset uploaded as the
> > reviewable
> > > >>  > change. Frequently a change to the first commit requires
> > > regenerating all
> > > >>  > of these patches and uploading them to the individual review
> pages.
> > > >>  >
> > > >>  > This gets even worse with larger chains of commits.
> > > >>  >
> > > >>  > It would be great if all of our changes could be small units of
> > > work, but
> > > >>  > very frequently we want to make sure we are ready to merge a
> > complete
> > > >>  > feature before starting the review process. We need to have a
> > better
> > > way
> > > >  to
> > > >>  > manage these large review units, as I do not see the possibility
> of
> > > >>  > breaking up the work into smaller units as a likely solution. We
> > > still
> > > >  have
> > > >>  > lots of features and system cleanup to work on.
> > > >>  >
> > > >>  > For anyone unfamiliar, github pull requests are based on a branch
> > you
> > > >  push
> > > >>  > to your personal fork. They give space for a general discussion,
> as
> > > well
> > > >  as
> > > >>  > allow commenting inline on the diff. They give a clear reference
> to
> > > each
> > > >>  > commit in the branch, allowing reviewers to see each piece of
> work
> > > >>  > individually as well as provide a "squashed" view to see the
> > overall
> > > >>  > differences.
> > > >>  >
> > > >>  > For the sake of keeping the project history connected to JIRA, we
> > > can see
> > > >>  > if there is enough automatic github integration or possibly
> upload
> > > patch
> > > >>  > files to JIRA each time we update a pull request. As an side
> note,
> > > if we
> > > >>  > don't need individual patches for reviewboard we could just put
> > patch
> > > >  files
> > > >>  > on JIRA that contain several commits. These are much easier to
> > > generate
> > > >  an
> > > >>  > apply than a bunch of individual files for each change. This
> should
> > > >  prevent
> > > >>  > JIRAs needing long lists of patches with names like
> > > >>  > DRILL-3000-part1-version3.patch
> > > >>  >
> > > >
> > > >
> > > >
> > > >  --
> > > >   Steven Phillips
> > > >   Software Engineer
> > > >
> > > >   mapr.com
> > > >
> > >
> > >
> > >
> > >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Hanifi Gunes <hg...@maprtech.com>.
+1

At the very least GitHub will be UP.

On Tue, Jun 23, 2015 at 2:18 PM, Parth Chandra <pc...@maprtech.com>
wrote:

> +1 on trying this. RB has been pretty painful to us.
>
>
>
> On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <ma...@gmail.com>
> wrote:
>
> > Is Travis <https://travis-ci.org/>  a viable option for the GitHub
> route?
> > I
> > use it for my own projects to build pull requests (with additional code
> > quality targets like CheckStyle, PMD, etc.). Perhaps that would take some
> > of
> > the burden off the reviewers and let them focus on the proposed
> > implementations, rather than some of the more tedious aspects of each
> > review.
> >
> > From:  Jacques Nadeau <ja...@apache.org>
> > Reply-To:  <de...@drill.apache.org>
> > Date:  Monday, June 22, 2015 at 10:22 PM
> > To:  "dev@drill.apache.org" <de...@drill.apache.org>
> > Subject:  Re: [DISCUSS] Allowing the option to use github pull requests
> in
> > place of reviewboard
> >
> > I'm up for this if we deprecate the old way.  Having two different
> > processes seems like overkill.  In general, I find the review interface
> of
> > GitHub less expressive/clear but everything else is way better.
> >
> > On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <sphillips@maprtech.com
> >
> > wrote:
> >
> > >  +1
> > >
> > >  I am in favor of giving this a try.
> > >
> > >  If I remember correctly, the reason we abandoned pull requests
> > originally
> > >  was because we couldn't close the pull requests through Github. A
> > solution
> > >  could be for whoever pushes the commit to the apache git repo to add
> the
> > >  Line "Closes <request number>". Github would then automatically close
> > the
> > >  pull request.
> > >
> > >  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <
> > altekrusejason@gmail.com
> > >>  >
> > >  wrote:
> > >
> > >>  > Hello Drill developers,
> > >>  >
> > >>  > I am writing this message today to propose allowing the use of
> github
> > >  pull
> > >>  > requests to perform reviews in place of the apache reviewboard
> > instance.
> > >>  >
> > >>  > Reviewboard has caused a number of headaches in the past few
> months,
> > and
> > >  I
> > >>  > think its time to evaluate the benefits of the apache
> infrastructure
> > >>  > relative to the actual cost of using it in practice.
> > >>  >
> > >>  > For clarity of the discussion, we cannot use the complete github
> > >  workflow.
> > >>  > Comitters will still need to use patch files, or check out the
> branch
> > >  used
> > >>  > in the review request and push to apache master manually. I am not
> > >>  > advocating for using a merging strategy with git, just for using
> the
> > >  github
> > >>  > web UI for reviews. I expect anyone generating a chain of commits
> as
> > >>  > described below to use the rebasing workflow we do today.
> > Additionally
> > >  devs
> > >>  > should only be breaking up work to make it easier to review, we
> will
> > not
> > >  be
> > >>  > reviewing branches that contain a bunch of useless WIP commits.
> > >>  >
> > >>  > A few examples of problems I have experienced with reviewboard
> > include:
> > >>  > corruption of patches when they are downloaded, the web interface
> > showing
> > >>  > inconsistent content from the raw diff, and random rejection of
> > patches
> > >>  > that are based directly on the head of apache master.
> > >>  >
> > >>  > These are all serious blockers for getting code reviewed and
> > integrated
> > >>  > into the master branch in a timely manner.
> > >>  >
> > >>  > In addition to serious bugs in reviewboard, there are a number of
> > >>  > difficulties with the combination of our typical dev workflow and
> how
> > >>  > reviewboard works with patches. As we are still adding features to
> > Drill,
> > >>  > we often have several weeks of work to submit in response to a JIRA
> > or
> > >>  > series of related JIRAs. Sometimes this work can be broken up into
> > >>  > independent reviewable units, and other times it cannot. When a
> > series of
> > >>  > changes requires a mixture of refactoring and additions, the
> process
> > is
> > >>  > currently quite painful. Ether reviewers need to look through a
> giant
> > >  messy
> > >>  > diff, or the submitters need to do a lot of extra work. This
> > involves not
> > >>  > only organizing their work into a reviewable series of commits, but
> > also
> > >>  > generating redundant squashed versions of the intermediate work to
> > make
> > >>  > reviewboard happy.
> > >>  >
> > >>  > For a relatively simple 3 part change, this involves creating 3
> > >  reviewboard
> > >>  > pages. The first will contain the first commit by itself. The
> second
> > will
> > >>  > have the first commits patch as a parent patch with the next change
> > in
> > >  the
> > >>  > series uploaded as the core change to review. For the third
> change, a
> > >>  > squashed version of the first two commits must be generated to
> serve
> > as a
> > >>  > parent patch and then the third changeset uploaded as the
> reviewable
> > >>  > change. Frequently a change to the first commit requires
> > regenerating all
> > >>  > of these patches and uploading them to the individual review pages.
> > >>  >
> > >>  > This gets even worse with larger chains of commits.
> > >>  >
> > >>  > It would be great if all of our changes could be small units of
> > work, but
> > >>  > very frequently we want to make sure we are ready to merge a
> complete
> > >>  > feature before starting the review process. We need to have a
> better
> > way
> > >  to
> > >>  > manage these large review units, as I do not see the possibility of
> > >>  > breaking up the work into smaller units as a likely solution. We
> > still
> > >  have
> > >>  > lots of features and system cleanup to work on.
> > >>  >
> > >>  > For anyone unfamiliar, github pull requests are based on a branch
> you
> > >  push
> > >>  > to your personal fork. They give space for a general discussion, as
> > well
> > >  as
> > >>  > allow commenting inline on the diff. They give a clear reference to
> > each
> > >>  > commit in the branch, allowing reviewers to see each piece of work
> > >>  > individually as well as provide a "squashed" view to see the
> overall
> > >>  > differences.
> > >>  >
> > >>  > For the sake of keeping the project history connected to JIRA, we
> > can see
> > >>  > if there is enough automatic github integration or possibly upload
> > patch
> > >>  > files to JIRA each time we update a pull request. As an side note,
> > if we
> > >>  > don't need individual patches for reviewboard we could just put
> patch
> > >  files
> > >>  > on JIRA that contain several commits. These are much easier to
> > generate
> > >  an
> > >>  > apply than a bunch of individual files for each change. This should
> > >  prevent
> > >>  > JIRAs needing long lists of patches with names like
> > >>  > DRILL-3000-part1-version3.patch
> > >>  >
> > >
> > >
> > >
> > >  --
> > >   Steven Phillips
> > >   Software Engineer
> > >
> > >   mapr.com
> > >
> >
> >
> >
> >
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Parth Chandra <pc...@maprtech.com>.
+1 on trying this. RB has been pretty painful to us.



On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <ma...@gmail.com>
wrote:

> Is Travis <https://travis-ci.org/>  a viable option for the GitHub route?
> I
> use it for my own projects to build pull requests (with additional code
> quality targets like CheckStyle, PMD, etc.). Perhaps that would take some
> of
> the burden off the reviewers and let them focus on the proposed
> implementations, rather than some of the more tedious aspects of each
> review.
>
> From:  Jacques Nadeau <ja...@apache.org>
> Reply-To:  <de...@drill.apache.org>
> Date:  Monday, June 22, 2015 at 10:22 PM
> To:  "dev@drill.apache.org" <de...@drill.apache.org>
> Subject:  Re: [DISCUSS] Allowing the option to use github pull requests in
> place of reviewboard
>
> I'm up for this if we deprecate the old way.  Having two different
> processes seems like overkill.  In general, I find the review interface of
> GitHub less expressive/clear but everything else is way better.
>
> On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <sp...@maprtech.com>
> wrote:
>
> >  +1
> >
> >  I am in favor of giving this a try.
> >
> >  If I remember correctly, the reason we abandoned pull requests
> originally
> >  was because we couldn't close the pull requests through Github. A
> solution
> >  could be for whoever pushes the commit to the apache git repo to add the
> >  Line "Closes <request number>". Github would then automatically close
> the
> >  pull request.
> >
> >  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <
> altekrusejason@gmail.com
> >>  >
> >  wrote:
> >
> >>  > Hello Drill developers,
> >>  >
> >>  > I am writing this message today to propose allowing the use of github
> >  pull
> >>  > requests to perform reviews in place of the apache reviewboard
> instance.
> >>  >
> >>  > Reviewboard has caused a number of headaches in the past few months,
> and
> >  I
> >>  > think its time to evaluate the benefits of the apache infrastructure
> >>  > relative to the actual cost of using it in practice.
> >>  >
> >>  > For clarity of the discussion, we cannot use the complete github
> >  workflow.
> >>  > Comitters will still need to use patch files, or check out the branch
> >  used
> >>  > in the review request and push to apache master manually. I am not
> >>  > advocating for using a merging strategy with git, just for using the
> >  github
> >>  > web UI for reviews. I expect anyone generating a chain of commits as
> >>  > described below to use the rebasing workflow we do today.
> Additionally
> >  devs
> >>  > should only be breaking up work to make it easier to review, we will
> not
> >  be
> >>  > reviewing branches that contain a bunch of useless WIP commits.
> >>  >
> >>  > A few examples of problems I have experienced with reviewboard
> include:
> >>  > corruption of patches when they are downloaded, the web interface
> showing
> >>  > inconsistent content from the raw diff, and random rejection of
> patches
> >>  > that are based directly on the head of apache master.
> >>  >
> >>  > These are all serious blockers for getting code reviewed and
> integrated
> >>  > into the master branch in a timely manner.
> >>  >
> >>  > In addition to serious bugs in reviewboard, there are a number of
> >>  > difficulties with the combination of our typical dev workflow and how
> >>  > reviewboard works with patches. As we are still adding features to
> Drill,
> >>  > we often have several weeks of work to submit in response to a JIRA
> or
> >>  > series of related JIRAs. Sometimes this work can be broken up into
> >>  > independent reviewable units, and other times it cannot. When a
> series of
> >>  > changes requires a mixture of refactoring and additions, the process
> is
> >>  > currently quite painful. Ether reviewers need to look through a giant
> >  messy
> >>  > diff, or the submitters need to do a lot of extra work. This
> involves not
> >>  > only organizing their work into a reviewable series of commits, but
> also
> >>  > generating redundant squashed versions of the intermediate work to
> make
> >>  > reviewboard happy.
> >>  >
> >>  > For a relatively simple 3 part change, this involves creating 3
> >  reviewboard
> >>  > pages. The first will contain the first commit by itself. The second
> will
> >>  > have the first commits patch as a parent patch with the next change
> in
> >  the
> >>  > series uploaded as the core change to review. For the third change, a
> >>  > squashed version of the first two commits must be generated to serve
> as a
> >>  > parent patch and then the third changeset uploaded as the reviewable
> >>  > change. Frequently a change to the first commit requires
> regenerating all
> >>  > of these patches and uploading them to the individual review pages.
> >>  >
> >>  > This gets even worse with larger chains of commits.
> >>  >
> >>  > It would be great if all of our changes could be small units of
> work, but
> >>  > very frequently we want to make sure we are ready to merge a complete
> >>  > feature before starting the review process. We need to have a better
> way
> >  to
> >>  > manage these large review units, as I do not see the possibility of
> >>  > breaking up the work into smaller units as a likely solution. We
> still
> >  have
> >>  > lots of features and system cleanup to work on.
> >>  >
> >>  > For anyone unfamiliar, github pull requests are based on a branch you
> >  push
> >>  > to your personal fork. They give space for a general discussion, as
> well
> >  as
> >>  > allow commenting inline on the diff. They give a clear reference to
> each
> >>  > commit in the branch, allowing reviewers to see each piece of work
> >>  > individually as well as provide a "squashed" view to see the overall
> >>  > differences.
> >>  >
> >>  > For the sake of keeping the project history connected to JIRA, we
> can see
> >>  > if there is enough automatic github integration or possibly upload
> patch
> >>  > files to JIRA each time we update a pull request. As an side note,
> if we
> >>  > don't need individual patches for reviewboard we could just put patch
> >  files
> >>  > on JIRA that contain several commits. These are much easier to
> generate
> >  an
> >>  > apply than a bunch of individual files for each change. This should
> >  prevent
> >>  > JIRAs needing long lists of patches with names like
> >>  > DRILL-3000-part1-version3.patch
> >>  >
> >
> >
> >
> >  --
> >   Steven Phillips
> >   Software Engineer
> >
> >   mapr.com
> >
>
>
>
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Matthew Burgess <ma...@gmail.com>.
Is Travis <https://travis-ci.org/>  a viable option for the GitHub route? I
use it for my own projects to build pull requests (with additional code
quality targets like CheckStyle, PMD, etc.). Perhaps that would take some of
the burden off the reviewers and let them focus on the proposed
implementations, rather than some of the more tedious aspects of each
review.

From:  Jacques Nadeau <ja...@apache.org>
Reply-To:  <de...@drill.apache.org>
Date:  Monday, June 22, 2015 at 10:22 PM
To:  "dev@drill.apache.org" <de...@drill.apache.org>
Subject:  Re: [DISCUSS] Allowing the option to use github pull requests in
place of reviewboard

I'm up for this if we deprecate the old way.  Having two different
processes seems like overkill.  In general, I find the review interface of
GitHub less expressive/clear but everything else is way better.

On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <sp...@maprtech.com>
wrote:

>  +1
> 
>  I am in favor of giving this a try.
> 
>  If I remember correctly, the reason we abandoned pull requests originally
>  was because we couldn't close the pull requests through Github. A solution
>  could be for whoever pushes the commit to the apache git repo to add the
>  Line "Closes <request number>". Github would then automatically close the
>  pull request.
> 
>  On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <altekrusejason@gmail.com
>>  >
>  wrote:
> 
>>  > Hello Drill developers,
>>  >
>>  > I am writing this message today to propose allowing the use of github
>  pull
>>  > requests to perform reviews in place of the apache reviewboard instance.
>>  >
>>  > Reviewboard has caused a number of headaches in the past few months, and
>  I
>>  > think its time to evaluate the benefits of the apache infrastructure
>>  > relative to the actual cost of using it in practice.
>>  >
>>  > For clarity of the discussion, we cannot use the complete github
>  workflow.
>>  > Comitters will still need to use patch files, or check out the branch
>  used
>>  > in the review request and push to apache master manually. I am not
>>  > advocating for using a merging strategy with git, just for using the
>  github
>>  > web UI for reviews. I expect anyone generating a chain of commits as
>>  > described below to use the rebasing workflow we do today. Additionally
>  devs
>>  > should only be breaking up work to make it easier to review, we will not
>  be
>>  > reviewing branches that contain a bunch of useless WIP commits.
>>  >
>>  > A few examples of problems I have experienced with reviewboard include:
>>  > corruption of patches when they are downloaded, the web interface showing
>>  > inconsistent content from the raw diff, and random rejection of patches
>>  > that are based directly on the head of apache master.
>>  >
>>  > These are all serious blockers for getting code reviewed and integrated
>>  > into the master branch in a timely manner.
>>  >
>>  > In addition to serious bugs in reviewboard, there are a number of
>>  > difficulties with the combination of our typical dev workflow and how
>>  > reviewboard works with patches. As we are still adding features to Drill,
>>  > we often have several weeks of work to submit in response to a JIRA or
>>  > series of related JIRAs. Sometimes this work can be broken up into
>>  > independent reviewable units, and other times it cannot. When a series of
>>  > changes requires a mixture of refactoring and additions, the process is
>>  > currently quite painful. Ether reviewers need to look through a giant
>  messy
>>  > diff, or the submitters need to do a lot of extra work. This involves not
>>  > only organizing their work into a reviewable series of commits, but also
>>  > generating redundant squashed versions of the intermediate work to make
>>  > reviewboard happy.
>>  >
>>  > For a relatively simple 3 part change, this involves creating 3
>  reviewboard
>>  > pages. The first will contain the first commit by itself. The second will
>>  > have the first commits patch as a parent patch with the next change in
>  the
>>  > series uploaded as the core change to review. For the third change, a
>>  > squashed version of the first two commits must be generated to serve as a
>>  > parent patch and then the third changeset uploaded as the reviewable
>>  > change. Frequently a change to the first commit requires regenerating all
>>  > of these patches and uploading them to the individual review pages.
>>  >
>>  > This gets even worse with larger chains of commits.
>>  >
>>  > It would be great if all of our changes could be small units of work, but
>>  > very frequently we want to make sure we are ready to merge a complete
>>  > feature before starting the review process. We need to have a better way
>  to
>>  > manage these large review units, as I do not see the possibility of
>>  > breaking up the work into smaller units as a likely solution. We still
>  have
>>  > lots of features and system cleanup to work on.
>>  >
>>  > For anyone unfamiliar, github pull requests are based on a branch you
>  push
>>  > to your personal fork. They give space for a general discussion, as well
>  as
>>  > allow commenting inline on the diff. They give a clear reference to each
>>  > commit in the branch, allowing reviewers to see each piece of work
>>  > individually as well as provide a "squashed" view to see the overall
>>  > differences.
>>  >
>>  > For the sake of keeping the project history connected to JIRA, we can see
>>  > if there is enough automatic github integration or possibly upload patch
>>  > files to JIRA each time we update a pull request. As an side note, if we
>>  > don't need individual patches for reviewboard we could just put patch
>  files
>>  > on JIRA that contain several commits. These are much easier to generate
>  an
>>  > apply than a bunch of individual files for each change. This should
>  prevent
>>  > JIRAs needing long lists of patches with names like
>>  > DRILL-3000-part1-version3.patch
>>  >
> 
> 
> 
>  --
>   Steven Phillips
>   Software Engineer
> 
>   mapr.com
> 




Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Ted Dunning <te...@gmail.com>.
On Mon, Jun 22, 2015 at 7:22 PM, Jacques Nadeau <ja...@apache.org> wrote:

> In general, I find the review interface of
> GitHub less expressive/clear but everything else is way better.
>

The trade-off is that more people can figure out how to put PR's up for
review than who can run reviewboard accurately.

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Jacques Nadeau <ja...@apache.org>.
I'm up for this if we deprecate the old way.  Having two different
processes seems like overkill.  In general, I find the review interface of
GitHub less expressive/clear but everything else is way better.

On Mon, Jun 22, 2015 at 6:59 PM, Steven Phillips <sp...@maprtech.com>
wrote:

> +1
>
> I am in favor of giving this a try.
>
> If I remember correctly, the reason we abandoned pull requests originally
> was because we couldn't close the pull requests through Github. A solution
> could be for whoever pushes the commit to the apache git repo to add the
> Line "Closes <request number>". Github would then automatically close the
> pull request.
>
> On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <altekrusejason@gmail.com
> >
> wrote:
>
> > Hello Drill developers,
> >
> > I am writing this message today to propose allowing the use of github
> pull
> > requests to perform reviews in place of the apache reviewboard instance.
> >
> > Reviewboard has caused a number of headaches in the past few months, and
> I
> > think its time to evaluate the benefits of the apache infrastructure
> > relative to the actual cost of using it in practice.
> >
> > For clarity of the discussion, we cannot use the complete github
> workflow.
> > Comitters will still need to use patch files, or check out the branch
> used
> > in the review request and push to apache master manually. I am not
> > advocating for using a merging strategy with git, just for using the
> github
> > web UI for reviews. I expect anyone generating a chain of commits as
> > described below to use the rebasing workflow we do today. Additionally
> devs
> > should only be breaking up work to make it easier to review, we will not
> be
> > reviewing branches that contain a bunch of useless WIP commits.
> >
> > A few examples of problems I have experienced with reviewboard include:
> > corruption of patches when they are downloaded, the web interface showing
> > inconsistent content from the raw diff, and random rejection of patches
> > that are based directly on the head of apache master.
> >
> > These are all serious blockers for getting code reviewed and integrated
> > into the master branch in a timely manner.
> >
> > In addition to serious bugs in reviewboard, there are a number of
> > difficulties with the combination of our typical dev workflow and how
> > reviewboard works with patches. As we are still adding features to Drill,
> > we often have several weeks of work to submit in response to a JIRA or
> > series of related JIRAs. Sometimes this work can be broken up into
> > independent reviewable units, and other times it cannot. When a series of
> > changes requires a mixture of refactoring and additions, the process is
> > currently quite painful. Ether reviewers need to look through a giant
> messy
> > diff, or the submitters need to do a lot of extra work. This involves not
> > only organizing their work into a reviewable series of commits, but also
> > generating redundant squashed versions of the intermediate work to make
> > reviewboard happy.
> >
> > For a relatively simple 3 part change, this involves creating 3
> reviewboard
> > pages. The first will contain the first commit by itself. The second will
> > have the first commits patch as a parent patch with the next change in
> the
> > series uploaded as the core change to review. For the third change, a
> > squashed version of the first two commits must be generated to serve as a
> > parent patch and then the third changeset uploaded as the reviewable
> > change. Frequently a change to the first commit requires regenerating all
> > of these patches and uploading them to the individual review pages.
> >
> > This gets even worse with larger chains of commits.
> >
> > It would be great if all of our changes could be small units of work, but
> > very frequently we want to make sure we are ready to merge a complete
> > feature before starting the review process. We need to have a better way
> to
> > manage these large review units, as I do not see the possibility of
> > breaking up the work into smaller units as a likely solution. We still
> have
> > lots of features and system cleanup to work on.
> >
> > For anyone unfamiliar, github pull requests are based on a branch you
> push
> > to your personal fork. They give space for a general discussion, as well
> as
> > allow commenting inline on the diff. They give a clear reference to each
> > commit in the branch, allowing reviewers to see each piece of work
> > individually as well as provide a "squashed" view to see the overall
> > differences.
> >
> > For the sake of keeping the project history connected to JIRA, we can see
> > if there is enough automatic github integration or possibly upload patch
> > files to JIRA each time we update a pull request. As an side note, if we
> > don't need individual patches for reviewboard we could just put patch
> files
> > on JIRA that contain several commits. These are much easier to generate
> an
> > apply than a bunch of individual files for each change. This should
> prevent
> > JIRAs needing long lists of patches with names like
> > DRILL-3000-part1-version3.patch
> >
>
>
>
> --
>  Steven Phillips
>  Software Engineer
>
>  mapr.com
>

Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard

Posted by Steven Phillips <sp...@maprtech.com>.
+1

I am in favor of giving this a try.

If I remember correctly, the reason we abandoned pull requests originally
was because we couldn't close the pull requests through Github. A solution
could be for whoever pushes the commit to the apache git repo to add the
Line "Closes <request number>". Github would then automatically close the
pull request.

On Mon, Jun 22, 2015 at 1:02 PM, Jason Altekruse <al...@gmail.com>
wrote:

> Hello Drill developers,
>
> I am writing this message today to propose allowing the use of github pull
> requests to perform reviews in place of the apache reviewboard instance.
>
> Reviewboard has caused a number of headaches in the past few months, and I
> think its time to evaluate the benefits of the apache infrastructure
> relative to the actual cost of using it in practice.
>
> For clarity of the discussion, we cannot use the complete github workflow.
> Comitters will still need to use patch files, or check out the branch used
> in the review request and push to apache master manually. I am not
> advocating for using a merging strategy with git, just for using the github
> web UI for reviews. I expect anyone generating a chain of commits as
> described below to use the rebasing workflow we do today. Additionally devs
> should only be breaking up work to make it easier to review, we will not be
> reviewing branches that contain a bunch of useless WIP commits.
>
> A few examples of problems I have experienced with reviewboard include:
> corruption of patches when they are downloaded, the web interface showing
> inconsistent content from the raw diff, and random rejection of patches
> that are based directly on the head of apache master.
>
> These are all serious blockers for getting code reviewed and integrated
> into the master branch in a timely manner.
>
> In addition to serious bugs in reviewboard, there are a number of
> difficulties with the combination of our typical dev workflow and how
> reviewboard works with patches. As we are still adding features to Drill,
> we often have several weeks of work to submit in response to a JIRA or
> series of related JIRAs. Sometimes this work can be broken up into
> independent reviewable units, and other times it cannot. When a series of
> changes requires a mixture of refactoring and additions, the process is
> currently quite painful. Ether reviewers need to look through a giant messy
> diff, or the submitters need to do a lot of extra work. This involves not
> only organizing their work into a reviewable series of commits, but also
> generating redundant squashed versions of the intermediate work to make
> reviewboard happy.
>
> For a relatively simple 3 part change, this involves creating 3 reviewboard
> pages. The first will contain the first commit by itself. The second will
> have the first commits patch as a parent patch with the next change in the
> series uploaded as the core change to review. For the third change, a
> squashed version of the first two commits must be generated to serve as a
> parent patch and then the third changeset uploaded as the reviewable
> change. Frequently a change to the first commit requires regenerating all
> of these patches and uploading them to the individual review pages.
>
> This gets even worse with larger chains of commits.
>
> It would be great if all of our changes could be small units of work, but
> very frequently we want to make sure we are ready to merge a complete
> feature before starting the review process. We need to have a better way to
> manage these large review units, as I do not see the possibility of
> breaking up the work into smaller units as a likely solution. We still have
> lots of features and system cleanup to work on.
>
> For anyone unfamiliar, github pull requests are based on a branch you push
> to your personal fork. They give space for a general discussion, as well as
> allow commenting inline on the diff. They give a clear reference to each
> commit in the branch, allowing reviewers to see each piece of work
> individually as well as provide a "squashed" view to see the overall
> differences.
>
> For the sake of keeping the project history connected to JIRA, we can see
> if there is enough automatic github integration or possibly upload patch
> files to JIRA each time we update a pull request. As an side note, if we
> don't need individual patches for reviewboard we could just put patch files
> on JIRA that contain several commits. These are much easier to generate an
> apply than a bunch of individual files for each change. This should prevent
> JIRAs needing long lists of patches with names like
> DRILL-3000-part1-version3.patch
>



-- 
 Steven Phillips
 Software Engineer

 mapr.com