Sounds like #2 has a clear lead. Couple of suggestions. 1. How do we know the reviewer to specify? In many cases, we informally know a likely person. Community members may not be sure. Or, can a committer watch for such cases and do the assignment?
2. As it turns out, JIRA does not send an e-mail notification when assigning someone as a Reviewer. Instead, perhaps include a brief comment, mentioning that person, to force JIRA to alert the Reviewer that they have a new item. As a follow up, anyone know if we can lobby Apache to add status states? That is, to add the states that Julian suggested? Typical workflow: Open -> In Progress -> Reviewable -> Ready to Commit -> Resolved. (Actually, if we include QA activities, the flow should be Resolved —> Verified. But, that is a different discussion.) The list of states (visible from clicking Resolve), includes some odd items such as “Staged” and “REMIND”, which seem like specialized states added for ad-hoc purposes... Thanks, - Paul > On Nov 29, 2016, at 9:23 AM, Julian Hyde <jhyde.apa...@gmail.com> wrote: > > 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 >>>
