drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jhyde.apa...@gmail.com>
Subject Re: [PROPOSAL] Apache Jira Workflow for Code Reviews
Date Tue, 29 Nov 2016 17:23:56 GMT
I like 2 also. 

Is the following variant of 2 possible in JIRA? Make the "ready to commit" flag into a status.
Thus status changes from "open" to "in progress" to "reviewable" to "ready to commit" (or
"approved").

The inability to search by assignee should not be a huge problem - it is in the interests
of the project to ensure that the number of cases in "reviewable" or "ready to commit" state
at any moment is small. 

And furthermore, the author of a bug often doesn't know or care who will review or commit
it. So the very notion of an assignee is suspect.

Julian

> On Nov 29, 2016, at 07:53, Aman Sinha <amansinha@apache.org> wrote:
> 
> My preference is for #2 because of the reasons you mention.  For #1 my
> recollection is the DrillCommitter was used at a time when there were only
> couple of committers.  Today there are many.  Changing to DrillCommitter
> will leave ambiguity about which committer should look at it.
> Additionally,  I feel that today we are not using the field 'Reviewer' for
> its correct purpose.  This argues for #2.
> 
> One note about #2 worth mentioning is that folks would have to change their
> search criteria  to 'assigned *OR reviewer*' to find out what is on their
> plate.
> 
>> On Mon, Nov 28, 2016 at 4:51 PM, Zelaine Fong <zfong@maprtech.com> wrote:
>> 
>> I'd like to propose a small change to the current process for code reviews
>> in Apache Drill.  We need this change because non-committers are becoming
>> more involved in code reviews.
>> 
>> The current process is described at
>> https://drill.apache.org/docs/apache-drill-contribution-
>> guidelines/#step-3:-get-your-code-reviewed-and-committed-to-the-project.
>> It assumes that the individuals doing code reviews are code committers.
>> Here's a brief summary of the process:
>> 
>> *Current Process:*
>> Currently, the workflow is based on updating the Status and Assignee
>> fields.  When a pull request has been posted and is ready for review, the
>> issue status is changed to "Reviewable" and the Assignee is changed to the
>> designated code reviewer.  If the review is completed and the reviewer is a
>> committer, the reviewer/committer will commit the change. If the code
>> reviewer has comments that require the contributor to address, the issue
>> status changes back to "In Progress" and the Assignee is changed back to
>> the contributor.
>> 
>> *Proposed Change:*
>> The proposed change is to address the case where another step may be needed
>> to commit changes, if the changes have been reviewed by a non-committer.
>> Here are a couple of proposals on how to address this.  In both cases, once
>> a review has been satisfactorily completed on a pull request, a committer
>> will take over, bless the changes based on the prior review comments, do a
>> +1 on the patch, and then commit.
>> 
>> 
>> *Proposal #1:*When a fix is ready to be reviewed, set the Assignee to
>> "DrillCommitter".
>> 
>> Pros: Simple.  User "DrillCommitter" already exists.
>> Cons: Similar to changing the Assignee to the code reviewer, you lose track
>> of who is the original contributor of an issue.  Yes, it's in the comment
>> history, but there is no automated/easy way to get this info.
>> 
>> *Proposal #2:*
>> Leave the Assignee field unchanged.  This is also means changing the
>> current process of changing the Assignee to the code reviewer.  Instead,
>> 
>> 1) Continue to set the Status to "Reviewable" to indicate that the fix is
>> ready for review, but set the Reviewer field to the designated code
>> reviewer.
>> 2) If changes are needed after code review, change the status of the issue
>> back to "In Progress" as today.  But again, don't change the Assignee.
>> 3) If the changes are acceptable, the reviewer applies the
>> "ready-to-commit" tag on the Jira.
>> 
>> Pros: Maintains Assignee field
>> Cons: Requires querying for the tag "ready-to-commit" to find commit-ready
>> fixes.
>> 
>> Please comment with your preferences for proposal 1 or 2, or if you have
>> other suggestions.
>> 
>> Thanks.
>> 
>> -- Zelaine
>> 

Mime
View raw message