You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Stamatis Zampetakis <za...@gmail.com> on 2019/07/15 15:39:09 UTC

Add "IN REVIEW" state in JIRA Workflow

Hello,

I was thinking that it would be helpful if we had an additional JIRA state
stating that the ticket is under ongoing review.

It would help to better monitor progress and provide more insights towards
the release.

Having an assigned reviewer (most often a committer) would mean that this
person is going to help resolving the ticket for the next release (or find
somebody else to delegate this task). That doesn't mean that other people
should not participate in the discussion, however if things block he/she
should be the first person to take action.

What do you think?

If there are not any objections, I will create an INFRA ticket.

Best,
Stamatis

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Michael Mior <mm...@apache.org>.
Thanks for clarifying Danny. I agree that what you suggest could be a good idea.

--
Michael Mior
mmior@apache.org

Le mar. 23 juil. 2019 à 21:50, Danny Chan <yu...@gmail.com> a écrit :
>
> Thanks Michael, just to make things more clear, I’m not saying only committers can review the code, I’m talking about the “IN REVIEW” tag, if we mark the PR in review, it would somehow discourage other people who has willingness on the code review. So I think It is not a good idea to let anyone change the tag state.
>
> Actually I just added a “Request Review” tag on the GitHub  page, because there are often some delay of code reviewing these days.
>
> Best,
> Danny Chan
> 在 2019年7月23日 +0800 PM9:45,Michael Mior <mm...@apache.org>,写道:
> > I would strongly oppose limiting who can review code. Only committers
> > can actually commit code, so we already have a mechanism for limiting
> > what code makes it in. I haven't seen anyone give a really bad code
> > review and if that does happen, I would rather address it on a case by
> > case basis instead of discouraging people from reviewing code.
> >
> > On the topic of making it easier to review PRs, what would be helpful
> > for me is if people could assign PRs to themselves on GitHub when they
> > agree to review them. It makes it a lot easier to find ones which are
> > languishing when I have a bit of spare time to review PRs. I think
> > this is a relatively easy process since it's just one extra click if
> > you're viewing the PR on GitHub.
> >
> > --
> > Michael Mior
> > mmior@apache.org
> >
> > Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a écrit :
> > >
> > > Sounds like a good idea if this state can only be seen by committers/PMC, because we should keep the quality of code reviewing, we should make some limit on who can review the code, as far as I know, many contributors are not that familiar with our code, and usually a good review comes from committers or even only from Julian !
> > >
> > > Best,
> > > Danny Chan
> > > 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > > > Hello,
> > > >
> > > > I was thinking that it would be helpful if we had an additional JIRA state
> > > > stating that the ticket is under ongoing review.
> > > >
> > > > It would help to better monitor progress and provide more insights towards
> > > > the release.
> > > >
> > > > Having an assigned reviewer (most often a committer) would mean that this
> > > > person is going to help resolving the ticket for the next release (or find
> > > > somebody else to delegate this task). That doesn't mean that other people
> > > > should not participate in the discussion, however if things block he/she
> > > > should be the first person to take action.
> > > >
> > > > What do you think?
> > > >
> > > > If there are not any objections, I will create an INFRA ticket.
> > > >
> > > > Best,
> > > > Stamatis

Re: Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Haisheng Yuan <h....@alibaba-inc.com>.
The intention of Stamatis's proposal is to just add a new state in JIRA workflow, to help clarify that the pull request is opened and under review.  It does help monitor the working progress, especially for a release manager, IMHO. But I don't think having an assigned reviewer is necessary.

- Haisheng

------------------------------------------------------------------
发件人:Danny Chan<yu...@gmail.com>
日 期:2019年07月24日 09:50:05
收件人:<de...@calcite.apache.org>
主 题:Re: Add "IN REVIEW" state in JIRA Workflow

Thanks Michael, just to make things more clear, I’m not saying only committers can review the code, I’m talking about the “IN REVIEW” tag, if we mark the PR in review, it would somehow discourage other people who has willingness on the code review. So I think It is not a good idea to let anyone change the tag state.

Actually I just added a “Request Review” tag on the GitHub  page, because there are often some delay of code reviewing these days.

Best,
Danny Chan
在 2019年7月23日 +0800 PM9:45,Michael Mior <mm...@apache.org>,写道:
> I would strongly oppose limiting who can review code. Only committers
> can actually commit code, so we already have a mechanism for limiting
> what code makes it in. I haven't seen anyone give a really bad code
> review and if that does happen, I would rather address it on a case by
> case basis instead of discouraging people from reviewing code.
>
> On the topic of making it easier to review PRs, what would be helpful
> for me is if people could assign PRs to themselves on GitHub when they
> agree to review them. It makes it a lot easier to find ones which are
> languishing when I have a bit of spare time to review PRs. I think
> this is a relatively easy process since it's just one extra click if
> you're viewing the PR on GitHub.
>
> --
> Michael Mior
> mmior@apache.org
>
> Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a écrit :
> >
> > Sounds like a good idea if this state can only be seen by committers/PMC, because we should keep the quality of code reviewing, we should make some limit on who can review the code, as far as I know, many contributors are not that familiar with our code, and usually a good review comes from committers or even only from Julian !
> >
> > Best,
> > Danny Chan
> > 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > > Hello,
> > >
> > > I was thinking that it would be helpful if we had an additional JIRA state
> > > stating that the ticket is under ongoing review.
> > >
> > > It would help to better monitor progress and provide more insights towards
> > > the release.
> > >
> > > Having an assigned reviewer (most often a committer) would mean that this
> > > person is going to help resolving the ticket for the next release (or find
> > > somebody else to delegate this task). That doesn't mean that other people
> > > should not participate in the discussion, however if things block he/she
> > > should be the first person to take action.
> > >
> > > What do you think?
> > >
> > > If there are not any objections, I will create an INFRA ticket.
> > >
> > > Best,
> > > Stamatis


Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Danny Chan <yu...@gmail.com>.
Thanks Michael, just to make things more clear, I’m not saying only committers can review the code, I’m talking about the “IN REVIEW” tag, if we mark the PR in review, it would somehow discourage other people who has willingness on the code review. So I think It is not a good idea to let anyone change the tag state.

Actually I just added a “Request Review” tag on the GitHub  page, because there are often some delay of code reviewing these days.

Best,
Danny Chan
在 2019年7月23日 +0800 PM9:45,Michael Mior <mm...@apache.org>,写道:
> I would strongly oppose limiting who can review code. Only committers
> can actually commit code, so we already have a mechanism for limiting
> what code makes it in. I haven't seen anyone give a really bad code
> review and if that does happen, I would rather address it on a case by
> case basis instead of discouraging people from reviewing code.
>
> On the topic of making it easier to review PRs, what would be helpful
> for me is if people could assign PRs to themselves on GitHub when they
> agree to review them. It makes it a lot easier to find ones which are
> languishing when I have a bit of spare time to review PRs. I think
> this is a relatively easy process since it's just one extra click if
> you're viewing the PR on GitHub.
>
> --
> Michael Mior
> mmior@apache.org
>
> Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a écrit :
> >
> > Sounds like a good idea if this state can only be seen by committers/PMC, because we should keep the quality of code reviewing, we should make some limit on who can review the code, as far as I know, many contributors are not that familiar with our code, and usually a good review comes from committers or even only from Julian !
> >
> > Best,
> > Danny Chan
> > 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > > Hello,
> > >
> > > I was thinking that it would be helpful if we had an additional JIRA state
> > > stating that the ticket is under ongoing review.
> > >
> > > It would help to better monitor progress and provide more insights towards
> > > the release.
> > >
> > > Having an assigned reviewer (most often a committer) would mean that this
> > > person is going to help resolving the ticket for the next release (or find
> > > somebody else to delegate this task). That doesn't mean that other people
> > > should not participate in the discussion, however if things block he/she
> > > should be the first person to take action.
> > >
> > > What do you think?
> > >
> > > If there are not any objections, I will create an INFRA ticket.
> > >
> > > Best,
> > > Stamatis

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Stamatis Zampetakis <za...@gmail.com>.
Everybody (committer or not) should be able to review and this shouldn't
change in any case.

JIRA is really made for issue tracking so I think it is more appropriate
compared to GitHub. It allows to perform powerful queries and helpful views
that is impossible to achieve with GitHub (I outlined various use cases
previously).

Furthermore, setting a reviewer wouldn't be more than one click away so it
is not more complicated than GitHub.

As we discussed in the past the canonical place for holding discussions and
high level reviews should be the JIRA case and this state would be an
additional step towards this direction.

How about trying it out? We do not really lose anything since people who
don't want to use it would be able to skip this state.

Best,
Stamatis


On Tue, Jul 23, 2019, 5:03 PM Chunwei Lei <ch...@gmail.com> wrote:

> Thanks for your proposal, Stamatis.
>
> IMHO, "IN REVIEW" state is not necessary since it somehow tells people this
> PR is under review by someone which may discourage others to review it for
> both committers and contributors.
>
> I do agree that what is really helpful is if people could assign PRs to
> themselves
> on GitHub when they agree to review them.
>
>
>
> Best,
> Chunwei
>
>
> On Tue, Jul 23, 2019 at 9:45 PM Michael Mior <mm...@apache.org> wrote:
>
> > I would strongly oppose limiting who can review code. Only committers
> > can actually commit code, so we already have a mechanism for limiting
> > what code makes it in. I haven't seen anyone give a really bad code
> > review and if that does happen, I would rather address it on a case by
> > case basis instead of discouraging people from reviewing code.
> >
> > On the topic of making it easier to review PRs, what would be helpful
> > for me is if people could assign PRs to themselves on GitHub when they
> > agree to review them. It makes it a lot easier to find ones which are
> > languishing when I have a bit of spare time to review PRs. I think
> > this is a relatively easy process since it's just one extra click if
> > you're viewing the PR on GitHub.
> >
> > --
> > Michael Mior
> > mmior@apache.org
> >
> > Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a
> écrit :
> > >
> > > Sounds like a good idea if this state can only be seen by
> > committers/PMC, because we should keep the quality of code reviewing, we
> > should make some limit on who can review the code, as far as I know, many
> > contributors are not that familiar with our code, and usually a good
> review
> > comes from committers or even only from Julian !
> > >
> > > Best,
> > > Danny Chan
> > > 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > > > Hello,
> > > >
> > > > I was thinking that it would be helpful if we had an additional JIRA
> > state
> > > > stating that the ticket is under ongoing review.
> > > >
> > > > It would help to better monitor progress and provide more insights
> > towards
> > > > the release.
> > > >
> > > > Having an assigned reviewer (most often a committer) would mean that
> > this
> > > > person is going to help resolving the ticket for the next release (or
> > find
> > > > somebody else to delegate this task). That doesn't mean that other
> > people
> > > > should not participate in the discussion, however if things block
> > he/she
> > > > should be the first person to take action.
> > > >
> > > > What do you think?
> > > >
> > > > If there are not any objections, I will create an INFRA ticket.
> > > >
> > > > Best,
> > > > Stamatis
> >
>

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Chunwei Lei <ch...@gmail.com>.
Thanks for your proposal, Stamatis.

IMHO, "IN REVIEW" state is not necessary since it somehow tells people this
PR is under review by someone which may discourage others to review it for
both committers and contributors.

I do agree that what is really helpful is if people could assign PRs to
themselves
on GitHub when they agree to review them.



Best,
Chunwei


On Tue, Jul 23, 2019 at 9:45 PM Michael Mior <mm...@apache.org> wrote:

> I would strongly oppose limiting who can review code. Only committers
> can actually commit code, so we already have a mechanism for limiting
> what code makes it in. I haven't seen anyone give a really bad code
> review and if that does happen, I would rather address it on a case by
> case basis instead of discouraging people from reviewing code.
>
> On the topic of making it easier to review PRs, what would be helpful
> for me is if people could assign PRs to themselves on GitHub when they
> agree to review them. It makes it a lot easier to find ones which are
> languishing when I have a bit of spare time to review PRs. I think
> this is a relatively easy process since it's just one extra click if
> you're viewing the PR on GitHub.
>
> --
> Michael Mior
> mmior@apache.org
>
> Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a écrit :
> >
> > Sounds like a good idea if this state can only be seen by
> committers/PMC, because we should keep the quality of code reviewing, we
> should make some limit on who can review the code, as far as I know, many
> contributors are not that familiar with our code, and usually a good review
> comes from committers or even only from Julian !
> >
> > Best,
> > Danny Chan
> > 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > > Hello,
> > >
> > > I was thinking that it would be helpful if we had an additional JIRA
> state
> > > stating that the ticket is under ongoing review.
> > >
> > > It would help to better monitor progress and provide more insights
> towards
> > > the release.
> > >
> > > Having an assigned reviewer (most often a committer) would mean that
> this
> > > person is going to help resolving the ticket for the next release (or
> find
> > > somebody else to delegate this task). That doesn't mean that other
> people
> > > should not participate in the discussion, however if things block
> he/she
> > > should be the first person to take action.
> > >
> > > What do you think?
> > >
> > > If there are not any objections, I will create an INFRA ticket.
> > >
> > > Best,
> > > Stamatis
>

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Michael Mior <mm...@apache.org>.
I would strongly oppose limiting who can review code. Only committers
can actually commit code, so we already have a mechanism for limiting
what code makes it in. I haven't seen anyone give a really bad code
review and if that does happen, I would rather address it on a case by
case basis instead of discouraging people from reviewing code.

On the topic of making it easier to review PRs, what would be helpful
for me is if people could assign PRs to themselves on GitHub when they
agree to review them. It makes it a lot easier to find ones which are
languishing when I have a bit of spare time to review PRs. I think
this is a relatively easy process since it's just one extra click if
you're viewing the PR on GitHub.

--
Michael Mior
mmior@apache.org

Le lun. 15 juil. 2019 à 22:19, Danny Chan <yu...@gmail.com> a écrit :
>
> Sounds like a good idea if this state can only be seen by committers/PMC, because we should keep the quality of code reviewing, we should make some limit on who can review the code, as far as I know, many contributors are not that familiar with our code, and usually a good review comes from committers or even only from Julian !
>
> Best,
> Danny Chan
> 在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> > Hello,
> >
> > I was thinking that it would be helpful if we had an additional JIRA state
> > stating that the ticket is under ongoing review.
> >
> > It would help to better monitor progress and provide more insights towards
> > the release.
> >
> > Having an assigned reviewer (most often a committer) would mean that this
> > person is going to help resolving the ticket for the next release (or find
> > somebody else to delegate this task). That doesn't mean that other people
> > should not participate in the discussion, however if things block he/she
> > should be the first person to take action.
> >
> > What do you think?
> >
> > If there are not any objections, I will create an INFRA ticket.
> >
> > Best,
> > Stamatis

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Danny Chan <yu...@gmail.com>.
Sounds like a good idea if this state can only be seen by committers/PMC, because we should keep the quality of code reviewing, we should make some limit on who can review the code, as far as I know, many contributors are not that familiar with our code, and usually a good review comes from committers or even only from Julian !

Best,
Danny Chan
在 2019年7月15日 +0800 PM11:47,Stamatis Zampetakis <za...@gmail.com>,写道:
> Hello,
>
> I was thinking that it would be helpful if we had an additional JIRA state
> stating that the ticket is under ongoing review.
>
> It would help to better monitor progress and provide more insights towards
> the release.
>
> Having an assigned reviewer (most often a committer) would mean that this
> person is going to help resolving the ticket for the next release (or find
> somebody else to delegate this task). That doesn't mean that other people
> should not participate in the discussion, however if things block he/she
> should be the first person to take action.
>
> What do you think?
>
> If there are not any objections, I will create an INFRA ticket.
>
> Best,
> Stamatis

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Stamatis Zampetakis <za...@gmail.com>.
Thanks for your comments Julian!

I would like to clarify that the new state is certainly not an obligation
for people to follow (as it is not an obligation for somebody to mark the
Jira as "IN PROGRESS").

Moreover, I was not thinking that somebody assigns cases to people but
rather that people will kindly propose themselves to help fulfilling the
release. Selecting in advance which cases somebody will review may turn out
to be a good idea as it was the preselection of release managers.

Concretely, I would like to have an extra state to be able to establish
Jira filters/views for doing the following.

As a reviewer:
 * identify which cases are not in review by somebody else, and choose
among them which I want to solve for the next release;
 * list all the cases that remain to be reviewed by me and visualise
relevant information concerning the state of the review.

As a release manager:
 * monitor the overall progress of the release (10% IN PROGRESS, 20% IN
REVIEW, 70% RESOLVED) and anticipate when the release will be ready;
 * identify which cases are not in review and try to find volunteers for
these cases;
 * foresee which cases are going to make it in the release (those with a
reviewer) to better invest my effort.
 * identify which cases have been resolved without a review and react if
the change

As a PMC member:
 * gather additional statistics of people really helping the project by
performing reviews which counts for getting the status of committer/PMC.

Often I become a picky guy so maybe the above are not that important; I
certainly don't want to make the life of people more difficult.

On Mon, Jul 15, 2019 at 7:47 PM Julian Hyde <jh...@apache.org> wrote:

> I don’t think another state is necessary/helpful. It seems like more
> process, when what we are suffering is lack of resources, not lack of
> process.
>
> I do believe that it is helpful to add a comment when I am reviewing - see
> e.g. https://issues.apache.org/jira/browse/CALCITE-3183 <
> https://issues.apache.org/jira/browse/CALCITE-3183>. And when I get
> distracted (which I often do), the contributor can ping me a few days later
> and say “hey, how’s the review going?”.
>
> We’re not very good at assigning reviewers to cases. Everyone is a
> volunteer, so we don’t assign reviewers, we leave it to people to assign
> themselves as reviewer. If a new state would help with that problem I could
> be convinced to support it.
>
> Julian
>
>
> > On Jul 15, 2019, at 8:39 AM, Stamatis Zampetakis <za...@gmail.com>
> wrote:
> >
> > Hello,
> >
> > I was thinking that it would be helpful if we had an additional JIRA
> state
> > stating that the ticket is under ongoing review.
> >
> > It would help to better monitor progress and provide more insights
> towards
> > the release.
> >
> > Having an assigned reviewer (most often a committer) would mean that this
> > person is going to help resolving the ticket for the next release (or
> find
> > somebody else to delegate this task). That doesn't mean that other people
> > should not participate in the discussion, however if things block he/she
> > should be the first person to take action.
> >
> > What do you think?
> >
> > If there are not any objections, I will create an INFRA ticket.
> >
> > Best,
> > Stamatis
>
>

Re: Add "IN REVIEW" state in JIRA Workflow

Posted by Julian Hyde <jh...@apache.org>.
I don’t think another state is necessary/helpful. It seems like more process, when what we are suffering is lack of resources, not lack of process.

I do believe that it is helpful to add a comment when I am reviewing - see e.g. https://issues.apache.org/jira/browse/CALCITE-3183 <https://issues.apache.org/jira/browse/CALCITE-3183>. And when I get distracted (which I often do), the contributor can ping me a few days later and say “hey, how’s the review going?”.

We’re not very good at assigning reviewers to cases. Everyone is a volunteer, so we don’t assign reviewers, we leave it to people to assign themselves as reviewer. If a new state would help with that problem I could be convinced to support it.

Julian


> On Jul 15, 2019, at 8:39 AM, Stamatis Zampetakis <za...@gmail.com> wrote:
> 
> Hello,
> 
> I was thinking that it would be helpful if we had an additional JIRA state
> stating that the ticket is under ongoing review.
> 
> It would help to better monitor progress and provide more insights towards
> the release.
> 
> Having an assigned reviewer (most often a committer) would mean that this
> person is going to help resolving the ticket for the next release (or find
> somebody else to delegate this task). That doesn't mean that other people
> should not participate in the discussion, however if things block he/she
> should be the first person to take action.
> 
> What do you think?
> 
> If there are not any objections, I will create an INFRA ticket.
> 
> Best,
> Stamatis