drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacques Nadeau <jacq...@apache.org>
Subject Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard
Date Tue, 23 Jun 2015 02:22:23 GMT
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>

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

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