drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aman Sinha <amansi...@apache.org>
Subject Re: [PROPOSAL] Apache Jira Workflow for Code Reviews
Date Tue, 29 Nov 2016 15:53:10 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message