drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matthew Burgess <mattyb...@gmail.com>
Subject Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard
Date Tue, 23 Jun 2015 04:45:12 GMT
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 <jacques@apache.org>
Reply-To:  <dev@drill.apache.org>
Date:  Monday, June 22, 2015 at 10:22 PM
To:  "dev@drill.apache.org" <dev@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
> 




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