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 <amansi...@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 <zf...@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 >>
