drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Westin <chriswesti...@gmail.com>
Subject Re: [DISCUSS] Allowing the option to use github pull requests in place of reviewboard
Date Wed, 24 Jun 2015 01:10:31 GMT
And I'll bet GitHub is a lot faster too.

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

> +1
>
> At the very least GitHub will be UP.
>
> On Tue, Jun 23, 2015 at 2:18 PM, Parth Chandra <pchandra@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 <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