drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Parth Chandra <pchan...@maprtech.com>
Subject Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard
Date Tue, 23 Jun 2015 21:18:04 GMT
+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 <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