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
