+1 on trying this. RB has been pretty painful to us.
On Mon, Jun 22, 2015 at 9:45 PM, Matthew Burgess <[email protected]> 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 <[email protected]> > Reply-To: <[email protected]> > Date: Monday, June 22, 2015 at 10:22 PM > To: "[email protected]" <[email protected]> > 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 <[email protected]> > 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 < > [email protected] > >> > > > 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 > > > > > >
