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 Wed, 24 Jun 2015 15:35:11 GMT
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 <ted.dunning@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 <chriswestin42@gmail.com>
> wrote:
>
> > 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