"Review workflow" changes to prevent "broken review" issues.

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

"Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov-2
Igniters,

Currently, according to
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-SubmittingforReview
,
contributor can ask for review by moving ticket to PATCH AVAILABLE state.

And, as far as I can see, this cause broken tickets issue.
Contributor can wait somebody who'll review his changes for a month or even
more.

I propose to change workflow and *make contributor responsible to find
reviewer*.
It's pretty easy to find a person able to review changes in most of cases.

1) You can check git history of files you modified and find persons with
expertise in this code
2) In case you have problems with such search you can always use
maintainers list (
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers
)

Thoughts?
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Vladimir Ozerov
We should keep in mind that contributor do not have to do anything, as this
is community. Instead, it is responsibility of the community to make sure
that contributions are processed in a timely manner, and that contributors
have positive experience with us.

So, -1 from my side.

On Mon, Jun 5, 2017 at 6:28 PM, Anton Vinogradov <[hidden email]> wrote:

> Igniters,
>
> Currently, according to
> https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-SubmittingforReview
> ,
> contributor can ask for review by moving ticket to PATCH AVAILABLE state.
>
> And, as far as I can see, this cause broken tickets issue.
> Contributor can wait somebody who'll review his changes for a month or even
> more.
>
> I propose to change workflow and *make contributor responsible to find
> reviewer*.
> It's pretty easy to find a person able to review changes in most of cases.
>
> 1) You can check git history of files you modified and find persons with
> expertise in this code
> 2) In case you have problems with such search you can always use
> maintainers list (
> https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> )
>
> Thoughts?
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Dmitriy Pavlov
In reply to this post by Anton Vinogradov-2
Hi Igniters, Anton,

Let’s imagine that development process as a chain of production stages
1) Developing patch by contributor
2) Review changes by committer

Reviews waiting too long time to be done at stage 2 may indicate that speed
(potential throughput) of step 2 is less than throughput at step 1. T2<T1

In terms of this model (inspired by Goldratt’s Theory of Constraints
(TOC)), I have a question:
Will this responsibility movement (to find appropriate reviewer to
contributor) help us to increase overall throughput?

If we agree constraint in terms of TOC is throughput T2, I suggest
following steps
- Increase the throughput T2 of the committers
- Reduce the load on the committers by improve quality of code after stage
1 given to review (pre review by non-committer, automatic review, code
inspections)

Best Regards,
Dmitriy Pavlov


пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:

> Igniters,
>
> Currently, according to
>
> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-SubmittingforReview
> ,
> contributor can ask for review by moving ticket to PATCH AVAILABLE state.
>
> And, as far as I can see, this cause broken tickets issue.
> Contributor can wait somebody who'll review his changes for a month or even
> more.
>
> I propose to change workflow and *make contributor responsible to find
> reviewer*.
> It's pretty easy to find a person able to review changes in most of cases.
>
> 1) You can check git history of files you modified and find persons with
> expertise in this code
> 2) In case you have problems with such search you can always use
> maintainers list (
>
> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> )
>
> Thoughts?
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov-2
Vova,

Contributors interested to make contributions and I propose them to use
*same* strategy as we (people inside the community) use.
"-1" will not solve this issue, but my "tips" will.

Dmitry,

The main problem here is that nobody notified that someone is waiting for
the review.
It's not a problem for me to provide tips or to make review, but it's
problem to periodically check is there somebody waiting.

Guys,
Let's try this approach, and I'm pretty sure it will help.

On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <[hidden email]> wrote:

> Hi Igniters, Anton,
>
> Let’s imagine that development process as a chain of production stages
> 1) Developing patch by contributor
> 2) Review changes by committer
>
> Reviews waiting too long time to be done at stage 2 may indicate that speed
> (potential throughput) of step 2 is less than throughput at step 1. T2<T1
>
> In terms of this model (inspired by Goldratt’s Theory of Constraints
> (TOC)), I have a question:
> Will this responsibility movement (to find appropriate reviewer to
> contributor) help us to increase overall throughput?
>
> If we agree constraint in terms of TOC is throughput T2, I suggest
> following steps
> - Increase the throughput T2 of the committers
> - Reduce the load on the committers by improve quality of code after stage
> 1 given to review (pre review by non-committer, automatic review, code
> inspections)
>
> Best Regards,
> Dmitriy Pavlov
>
>
> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
>
> > Igniters,
> >
> > Currently, according to
> >
> > https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-SubmittingforReview
> > ,
> > contributor can ask for review by moving ticket to PATCH AVAILABLE state.
> >
> > And, as far as I can see, this cause broken tickets issue.
> > Contributor can wait somebody who'll review his changes for a month or
> even
> > more.
> >
> > I propose to change workflow and *make contributor responsible to find
> > reviewer*.
> > It's pretty easy to find a person able to review changes in most of
> cases.
> >
> > 1) You can check git history of files you modified and find persons with
> > expertise in this code
> > 2) In case you have problems with such search you can always use
> > maintainers list (
> >
> > https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > )
> >
> > Thoughts?
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Dmitriy Pavlov
Hi Anton,


It is ok for me if it is advice and hint for faster review, as it is now.


We can periodically remind about this opportunity at dev list or in the
issue comments. We can remind that tasks in patch available status may be
reassigned by contributor.


Looking from prospective of overall throughput: it is not clear for me how
this process change will help.


Best Regards,

Dmitriy Pavlov

пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:

> Vova,
>
> Contributors interested to make contributions and I propose them to use
> *same* strategy as we (people inside the community) use.
> "-1" will not solve this issue, but my "tips" will.
>
> Dmitry,
>
> The main problem here is that nobody notified that someone is waiting for
> the review.
> It's not a problem for me to provide tips or to make review, but it's
> problem to periodically check is there somebody waiting.
>
> Guys,
> Let's try this approach, and I'm pretty sure it will help.
>
> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <[hidden email]>
> wrote:
>
> > Hi Igniters, Anton,
> >
> > Let’s imagine that development process as a chain of production stages
> > 1) Developing patch by contributor
> > 2) Review changes by committer
> >
> > Reviews waiting too long time to be done at stage 2 may indicate that
> speed
> > (potential throughput) of step 2 is less than throughput at step 1. T2<T1
> >
> > In terms of this model (inspired by Goldratt’s Theory of Constraints
> > (TOC)), I have a question:
> > Will this responsibility movement (to find appropriate reviewer to
> > contributor) help us to increase overall throughput?
> >
> > If we agree constraint in terms of TOC is throughput T2, I suggest
> > following steps
> > - Increase the throughput T2 of the committers
> > - Reduce the load on the committers by improve quality of code after
> stage
> > 1 given to review (pre review by non-committer, automatic review, code
> > inspections)
> >
> > Best Regards,
> > Dmitriy Pavlov
> >
> >
> > пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> >
> > > Igniters,
> > >
> > > Currently, according to
> > >
> > > https://cwiki.apache.org/confluence/display/IGNITE/How+
> > to+Contribute#HowtoContribute-SubmittingforReview
> > > ,
> > > contributor can ask for review by moving ticket to PATCH AVAILABLE
> state.
> > >
> > > And, as far as I can see, this cause broken tickets issue.
> > > Contributor can wait somebody who'll review his changes for a month or
> > even
> > > more.
> > >
> > > I propose to change workflow and *make contributor responsible to find
> > > reviewer*.
> > > It's pretty easy to find a person able to review changes in most of
> > cases.
> > >
> > > 1) You can check git history of files you modified and find persons
> with
> > > expertise in this code
> > > 2) In case you have problems with such search you can always use
> > > maintainers list (
> > >
> > > https://cwiki.apache.org/confluence/display/IGNITE/How+
> > to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > > )
> > >
> > > Thoughts?
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

dmagda
In general, I tend to agree with Anton that something needs to be changed in this direction.

How many of you flip through dev list, JIRA or GitHub notifications in an attempt to find tickets that demand your attention? I bet the percentage is pretty low.

To solve this issue I see two options:
1) Proposed by Anton.
2) Having a guy in the community who’ll keep an eye on all the incoming pull-requests shuffling them between committer in the same way proposed in 1.

Personally, I’m for 1.


Denis

> On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <[hidden email]> wrote:
>
> Hi Anton,
>
>
> It is ok for me if it is advice and hint for faster review, as it is now.
>
>
> We can periodically remind about this opportunity at dev list or in the
> issue comments. We can remind that tasks in patch available status may be
> reassigned by contributor.
>
>
> Looking from prospective of overall throughput: it is not clear for me how
> this process change will help.
>
>
> Best Regards,
>
> Dmitriy Pavlov
>
> пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
>
>> Vova,
>>
>> Contributors interested to make contributions and I propose them to use
>> *same* strategy as we (people inside the community) use.
>> "-1" will not solve this issue, but my "tips" will.
>>
>> Dmitry,
>>
>> The main problem here is that nobody notified that someone is waiting for
>> the review.
>> It's not a problem for me to provide tips or to make review, but it's
>> problem to periodically check is there somebody waiting.
>>
>> Guys,
>> Let's try this approach, and I'm pretty sure it will help.
>>
>> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <[hidden email]>
>> wrote:
>>
>>> Hi Igniters, Anton,
>>>
>>> Let’s imagine that development process as a chain of production stages
>>> 1) Developing patch by contributor
>>> 2) Review changes by committer
>>>
>>> Reviews waiting too long time to be done at stage 2 may indicate that
>> speed
>>> (potential throughput) of step 2 is less than throughput at step 1. T2<T1
>>>
>>> In terms of this model (inspired by Goldratt’s Theory of Constraints
>>> (TOC)), I have a question:
>>> Will this responsibility movement (to find appropriate reviewer to
>>> contributor) help us to increase overall throughput?
>>>
>>> If we agree constraint in terms of TOC is throughput T2, I suggest
>>> following steps
>>> - Increase the throughput T2 of the committers
>>> - Reduce the load on the committers by improve quality of code after
>> stage
>>> 1 given to review (pre review by non-committer, automatic review, code
>>> inspections)
>>>
>>> Best Regards,
>>> Dmitriy Pavlov
>>>
>>>
>>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
>>>
>>>> Igniters,
>>>>
>>>> Currently, according to
>>>>
>>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>>> to+Contribute#HowtoContribute-SubmittingforReview
>>>> ,
>>>> contributor can ask for review by moving ticket to PATCH AVAILABLE
>> state.
>>>>
>>>> And, as far as I can see, this cause broken tickets issue.
>>>> Contributor can wait somebody who'll review his changes for a month or
>>> even
>>>> more.
>>>>
>>>> I propose to change workflow and *make contributor responsible to find
>>>> reviewer*.
>>>> It's pretty easy to find a person able to review changes in most of
>>> cases.
>>>>
>>>> 1) You can check git history of files you modified and find persons
>> with
>>>> expertise in this code
>>>> 2) In case you have problems with such search you can always use
>>>> maintainers list (
>>>>
>>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
>>>> )
>>>>
>>>> Thoughts?
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Igor Sapego
By the way, there are emails being sent from Jira to devlist every
time someone adds comment to ticket, or, for example, edits its
title which helps a lot to keep a track of ticket's state.

But by some reason there is no such notification when ticket silently
getting moved to "Patch available" state. I believe, that would help if
there was a notification for that. Is it possible to configure?

Best Regards,
Igor

On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]> wrote:

> In general, I tend to agree with Anton that something needs to be changed
> in this direction.
>
> How many of you flip through dev list, JIRA or GitHub notifications in an
> attempt to find tickets that demand your attention? I bet the percentage is
> pretty low.
>
> To solve this issue I see two options:
> 1) Proposed by Anton.
> 2) Having a guy in the community who’ll keep an eye on all the incoming
> pull-requests shuffling them between committer in the same way proposed in
> 1.
>
> Personally, I’m for 1.
>
> —
> Denis
>
> > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <[hidden email]>
> wrote:
> >
> > Hi Anton,
> >
> >
> > It is ok for me if it is advice and hint for faster review, as it is now.
> >
> >
> > We can periodically remind about this opportunity at dev list or in the
> > issue comments. We can remind that tasks in patch available status may be
> > reassigned by contributor.
> >
> >
> > Looking from prospective of overall throughput: it is not clear for me
> how
> > this process change will help.
> >
> >
> > Best Regards,
> >
> > Dmitriy Pavlov
> >
> > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> >
> >> Vova,
> >>
> >> Contributors interested to make contributions and I propose them to use
> >> *same* strategy as we (people inside the community) use.
> >> "-1" will not solve this issue, but my "tips" will.
> >>
> >> Dmitry,
> >>
> >> The main problem here is that nobody notified that someone is waiting
> for
> >> the review.
> >> It's not a problem for me to provide tips or to make review, but it's
> >> problem to periodically check is there somebody waiting.
> >>
> >> Guys,
> >> Let's try this approach, and I'm pretty sure it will help.
> >>
> >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <[hidden email]>
> >> wrote:
> >>
> >>> Hi Igniters, Anton,
> >>>
> >>> Let’s imagine that development process as a chain of production stages
> >>> 1) Developing patch by contributor
> >>> 2) Review changes by committer
> >>>
> >>> Reviews waiting too long time to be done at stage 2 may indicate that
> >> speed
> >>> (potential throughput) of step 2 is less than throughput at step 1.
> T2<T1
> >>>
> >>> In terms of this model (inspired by Goldratt’s Theory of Constraints
> >>> (TOC)), I have a question:
> >>> Will this responsibility movement (to find appropriate reviewer to
> >>> contributor) help us to increase overall throughput?
> >>>
> >>> If we agree constraint in terms of TOC is throughput T2, I suggest
> >>> following steps
> >>> - Increase the throughput T2 of the committers
> >>> - Reduce the load on the committers by improve quality of code after
> >> stage
> >>> 1 given to review (pre review by non-committer, automatic review, code
> >>> inspections)
> >>>
> >>> Best Regards,
> >>> Dmitriy Pavlov
> >>>
> >>>
> >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> >>>
> >>>> Igniters,
> >>>>
> >>>> Currently, according to
> >>>>
> >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >>> to+Contribute#HowtoContribute-SubmittingforReview
> >>>> ,
> >>>> contributor can ask for review by moving ticket to PATCH AVAILABLE
> >> state.
> >>>>
> >>>> And, as far as I can see, this cause broken tickets issue.
> >>>> Contributor can wait somebody who'll review his changes for a month or
> >>> even
> >>>> more.
> >>>>
> >>>> I propose to change workflow and *make contributor responsible to find
> >>>> reviewer*.
> >>>> It's pretty easy to find a person able to review changes in most of
> >>> cases.
> >>>>
> >>>> 1) You can check git history of files you modified and find persons
> >> with
> >>>> expertise in this code
> >>>> 2) In case you have problems with such search you can always use
> >>>> maintainers list (
> >>>>
> >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> >>>> )
> >>>>
> >>>> Thoughts?
> >>>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Dmitriy Pavlov
1) There is waiting for review list here
https://cwiki.apache.org/confluence/display/IGNITE/Issues+waiting+for+review

2) some of contributions are supplemented by dev-list messages (please
review my PR…)


And these two tools sometimes do not help. I suppose it is because of
reviewers already have some things to do, but not because of lack of tool
support. Do you have other explanations?


But still, Igor’s suggestion to notify to dev list sounds reasonable to me.



Anton, could it solve your requirement to know about issue needed to review?

пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:

> By the way, there are emails being sent from Jira to devlist every
> time someone adds comment to ticket, or, for example, edits its
> title which helps a lot to keep a track of ticket's state.
>
> But by some reason there is no such notification when ticket silently
> getting moved to "Patch available" state. I believe, that would help if
> there was a notification for that. Is it possible to configure?
>
> Best Regards,
> Igor
>
> On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]> wrote:
>
> > In general, I tend to agree with Anton that something needs to be changed
> > in this direction.
> >
> > How many of you flip through dev list, JIRA or GitHub notifications in an
> > attempt to find tickets that demand your attention? I bet the percentage
> is
> > pretty low.
> >
> > To solve this issue I see two options:
> > 1) Proposed by Anton.
> > 2) Having a guy in the community who’ll keep an eye on all the incoming
> > pull-requests shuffling them between committer in the same way proposed
> in
> > 1.
> >
> > Personally, I’m for 1.
> >
> > —
> > Denis
> >
> > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <[hidden email]>
> > wrote:
> > >
> > > Hi Anton,
> > >
> > >
> > > It is ok for me if it is advice and hint for faster review, as it is
> now.
> > >
> > >
> > > We can periodically remind about this opportunity at dev list or in the
> > > issue comments. We can remind that tasks in patch available status may
> be
> > > reassigned by contributor.
> > >
> > >
> > > Looking from prospective of overall throughput: it is not clear for me
> > how
> > > this process change will help.
> > >
> > >
> > > Best Regards,
> > >
> > > Dmitriy Pavlov
> > >
> > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> > >
> > >> Vova,
> > >>
> > >> Contributors interested to make contributions and I propose them to
> use
> > >> *same* strategy as we (people inside the community) use.
> > >> "-1" will not solve this issue, but my "tips" will.
> > >>
> > >> Dmitry,
> > >>
> > >> The main problem here is that nobody notified that someone is waiting
> > for
> > >> the review.
> > >> It's not a problem for me to provide tips or to make review, but it's
> > >> problem to periodically check is there somebody waiting.
> > >>
> > >> Guys,
> > >> Let's try this approach, and I'm pretty sure it will help.
> > >>
> > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <[hidden email]>
> > >> wrote:
> > >>
> > >>> Hi Igniters, Anton,
> > >>>
> > >>> Let’s imagine that development process as a chain of production
> stages
> > >>> 1) Developing patch by contributor
> > >>> 2) Review changes by committer
> > >>>
> > >>> Reviews waiting too long time to be done at stage 2 may indicate that
> > >> speed
> > >>> (potential throughput) of step 2 is less than throughput at step 1.
> > T2<T1
> > >>>
> > >>> In terms of this model (inspired by Goldratt’s Theory of Constraints
> > >>> (TOC)), I have a question:
> > >>> Will this responsibility movement (to find appropriate reviewer to
> > >>> contributor) help us to increase overall throughput?
> > >>>
> > >>> If we agree constraint in terms of TOC is throughput T2, I suggest
> > >>> following steps
> > >>> - Increase the throughput T2 of the committers
> > >>> - Reduce the load on the committers by improve quality of code after
> > >> stage
> > >>> 1 given to review (pre review by non-committer, automatic review,
> code
> > >>> inspections)
> > >>>
> > >>> Best Regards,
> > >>> Dmitriy Pavlov
> > >>>
> > >>>
> > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> > >>>
> > >>>> Igniters,
> > >>>>
> > >>>> Currently, according to
> > >>>>
> > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > >>>> ,
> > >>>> contributor can ask for review by moving ticket to PATCH AVAILABLE
> > >> state.
> > >>>>
> > >>>> And, as far as I can see, this cause broken tickets issue.
> > >>>> Contributor can wait somebody who'll review his changes for a month
> or
> > >>> even
> > >>>> more.
> > >>>>
> > >>>> I propose to change workflow and *make contributor responsible to
> find
> > >>>> reviewer*.
> > >>>> It's pretty easy to find a person able to review changes in most of
> > >>> cases.
> > >>>>
> > >>>> 1) You can check git history of files you modified and find persons
> > >> with
> > >>>> expertise in this code
> > >>>> 2) In case you have problems with such search you can always use
> > >>>> maintainers list (
> > >>>>
> > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > >>>> )
> > >>>>
> > >>>> Thoughts?
> > >>>>
> > >>>
> > >>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

dsetrakyan
Wow, we have 66 tickets waiting for review.... this is pretty bad.
Something must definitely change.

From my side, having a contributor shop around for a reviewer is not really
fair. However, I would support the idea of Apache Ignite community electing
a person responsible to find reviewers for all contributions.

D.

On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <[hidden email]>
wrote:

> 1) There is waiting for review list here
> https://cwiki.apache.org/confluence/display/IGNITE/
> Issues+waiting+for+review
>
> 2) some of contributions are supplemented by dev-list messages (please
> review my PR…)
>
>
> And these two tools sometimes do not help. I suppose it is because of
> reviewers already have some things to do, but not because of lack of tool
> support. Do you have other explanations?
>
>
> But still, Igor’s suggestion to notify to dev list sounds reasonable to me.
>
>
>
> Anton, could it solve your requirement to know about issue needed to
> review?
>
> пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
>
> > By the way, there are emails being sent from Jira to devlist every
> > time someone adds comment to ticket, or, for example, edits its
> > title which helps a lot to keep a track of ticket's state.
> >
> > But by some reason there is no such notification when ticket silently
> > getting moved to "Patch available" state. I believe, that would help if
> > there was a notification for that. Is it possible to configure?
> >
> > Best Regards,
> > Igor
> >
> > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]> wrote:
> >
> > > In general, I tend to agree with Anton that something needs to be
> changed
> > > in this direction.
> > >
> > > How many of you flip through dev list, JIRA or GitHub notifications in
> an
> > > attempt to find tickets that demand your attention? I bet the
> percentage
> > is
> > > pretty low.
> > >
> > > To solve this issue I see two options:
> > > 1) Proposed by Anton.
> > > 2) Having a guy in the community who’ll keep an eye on all the incoming
> > > pull-requests shuffling them between committer in the same way proposed
> > in
> > > 1.
> > >
> > > Personally, I’m for 1.
> > >
> > > —
> > > Denis
> > >
> > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <[hidden email]>
> > > wrote:
> > > >
> > > > Hi Anton,
> > > >
> > > >
> > > > It is ok for me if it is advice and hint for faster review, as it is
> > now.
> > > >
> > > >
> > > > We can periodically remind about this opportunity at dev list or in
> the
> > > > issue comments. We can remind that tasks in patch available status
> may
> > be
> > > > reassigned by contributor.
> > > >
> > > >
> > > > Looking from prospective of overall throughput: it is not clear for
> me
> > > how
> > > > this process change will help.
> > > >
> > > >
> > > > Best Regards,
> > > >
> > > > Dmitriy Pavlov
> > > >
> > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> > > >
> > > >> Vova,
> > > >>
> > > >> Contributors interested to make contributions and I propose them to
> > use
> > > >> *same* strategy as we (people inside the community) use.
> > > >> "-1" will not solve this issue, but my "tips" will.
> > > >>
> > > >> Dmitry,
> > > >>
> > > >> The main problem here is that nobody notified that someone is
> waiting
> > > for
> > > >> the review.
> > > >> It's not a problem for me to provide tips or to make review, but
> it's
> > > >> problem to periodically check is there somebody waiting.
> > > >>
> > > >> Guys,
> > > >> Let's try this approach, and I'm pretty sure it will help.
> > > >>
> > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> [hidden email]>
> > > >> wrote:
> > > >>
> > > >>> Hi Igniters, Anton,
> > > >>>
> > > >>> Let’s imagine that development process as a chain of production
> > stages
> > > >>> 1) Developing patch by contributor
> > > >>> 2) Review changes by committer
> > > >>>
> > > >>> Reviews waiting too long time to be done at stage 2 may indicate
> that
> > > >> speed
> > > >>> (potential throughput) of step 2 is less than throughput at step 1.
> > > T2<T1
> > > >>>
> > > >>> In terms of this model (inspired by Goldratt’s Theory of
> Constraints
> > > >>> (TOC)), I have a question:
> > > >>> Will this responsibility movement (to find appropriate reviewer to
> > > >>> contributor) help us to increase overall throughput?
> > > >>>
> > > >>> If we agree constraint in terms of TOC is throughput T2, I suggest
> > > >>> following steps
> > > >>> - Increase the throughput T2 of the committers
> > > >>> - Reduce the load on the committers by improve quality of code
> after
> > > >> stage
> > > >>> 1 given to review (pre review by non-committer, automatic review,
> > code
> > > >>> inspections)
> > > >>>
> > > >>> Best Regards,
> > > >>> Dmitriy Pavlov
> > > >>>
> > > >>>
> > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> > > >>>
> > > >>>> Igniters,
> > > >>>>
> > > >>>> Currently, according to
> > > >>>>
> > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > > >>>> ,
> > > >>>> contributor can ask for review by moving ticket to PATCH AVAILABLE
> > > >> state.
> > > >>>>
> > > >>>> And, as far as I can see, this cause broken tickets issue.
> > > >>>> Contributor can wait somebody who'll review his changes for a
> month
> > or
> > > >>> even
> > > >>>> more.
> > > >>>>
> > > >>>> I propose to change workflow and *make contributor responsible to
> > find
> > > >>>> reviewer*.
> > > >>>> It's pretty easy to find a person able to review changes in most
> of
> > > >>> cases.
> > > >>>>
> > > >>>> 1) You can check git history of files you modified and find
> persons
> > > >> with
> > > >>>> expertise in this code
> > > >>>> 2) In case you have problems with such search you can always use
> > > >>>> maintainers list (
> > > >>>>
> > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > > >>>> )
> > > >>>>
> > > >>>> Thoughts?
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov
Dmitry Pavlov,

There is *HUGE *difference between "Devlist, please review my changes"
and "Dmitry Pavlov, please review my changes".

In case you're busy right now, you'll, most likely, ignore appeal to
devlist, but, I'm pretty sure, you'll check appeal to yourself.
Am I right?

So, my idea is: appeal to devlist is a useless spam maker approach, but
appeal to person(s) works.

On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Wow, we have 66 tickets waiting for review.... this is pretty bad.
> Something must definitely change.
>
> From my side, having a contributor shop around for a reviewer is not really
> fair. However, I would support the idea of Apache Ignite community electing
> a person responsible to find reviewers for all contributions.
>
> D.
>
> On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <[hidden email]>
> wrote:
>
> > 1) There is waiting for review list here
> > https://cwiki.apache.org/confluence/display/IGNITE/
> > Issues+waiting+for+review
> >
> > 2) some of contributions are supplemented by dev-list messages (please
> > review my PR…)
> >
> >
> > And these two tools sometimes do not help. I suppose it is because of
> > reviewers already have some things to do, but not because of lack of tool
> > support. Do you have other explanations?
> >
> >
> > But still, Igor’s suggestion to notify to dev list sounds reasonable to
> me.
> >
> >
> >
> > Anton, could it solve your requirement to know about issue needed to
> > review?
> >
> > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
> >
> > > By the way, there are emails being sent from Jira to devlist every
> > > time someone adds comment to ticket, or, for example, edits its
> > > title which helps a lot to keep a track of ticket's state.
> > >
> > > But by some reason there is no such notification when ticket silently
> > > getting moved to "Patch available" state. I believe, that would help if
> > > there was a notification for that. Is it possible to configure?
> > >
> > > Best Regards,
> > > Igor
> > >
> > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]> wrote:
> > >
> > > > In general, I tend to agree with Anton that something needs to be
> > changed
> > > > in this direction.
> > > >
> > > > How many of you flip through dev list, JIRA or GitHub notifications
> in
> > an
> > > > attempt to find tickets that demand your attention? I bet the
> > percentage
> > > is
> > > > pretty low.
> > > >
> > > > To solve this issue I see two options:
> > > > 1) Proposed by Anton.
> > > > 2) Having a guy in the community who’ll keep an eye on all the
> incoming
> > > > pull-requests shuffling them between committer in the same way
> proposed
> > > in
> > > > 1.
> > > >
> > > > Personally, I’m for 1.
> > > >
> > > > —
> > > > Denis
> > > >
> > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <[hidden email]>
> > > > wrote:
> > > > >
> > > > > Hi Anton,
> > > > >
> > > > >
> > > > > It is ok for me if it is advice and hint for faster review, as it
> is
> > > now.
> > > > >
> > > > >
> > > > > We can periodically remind about this opportunity at dev list or in
> > the
> > > > > issue comments. We can remind that tasks in patch available status
> > may
> > > be
> > > > > reassigned by contributor.
> > > > >
> > > > >
> > > > > Looking from prospective of overall throughput: it is not clear for
> > me
> > > > how
> > > > > this process change will help.
> > > > >
> > > > >
> > > > > Best Regards,
> > > > >
> > > > > Dmitriy Pavlov
> > > > >
> > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> > > > >
> > > > >> Vova,
> > > > >>
> > > > >> Contributors interested to make contributions and I propose them
> to
> > > use
> > > > >> *same* strategy as we (people inside the community) use.
> > > > >> "-1" will not solve this issue, but my "tips" will.
> > > > >>
> > > > >> Dmitry,
> > > > >>
> > > > >> The main problem here is that nobody notified that someone is
> > waiting
> > > > for
> > > > >> the review.
> > > > >> It's not a problem for me to provide tips or to make review, but
> > it's
> > > > >> problem to periodically check is there somebody waiting.
> > > > >>
> > > > >> Guys,
> > > > >> Let's try this approach, and I'm pretty sure it will help.
> > > > >>
> > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> > [hidden email]>
> > > > >> wrote:
> > > > >>
> > > > >>> Hi Igniters, Anton,
> > > > >>>
> > > > >>> Let’s imagine that development process as a chain of production
> > > stages
> > > > >>> 1) Developing patch by contributor
> > > > >>> 2) Review changes by committer
> > > > >>>
> > > > >>> Reviews waiting too long time to be done at stage 2 may indicate
> > that
> > > > >> speed
> > > > >>> (potential throughput) of step 2 is less than throughput at step
> 1.
> > > > T2<T1
> > > > >>>
> > > > >>> In terms of this model (inspired by Goldratt’s Theory of
> > Constraints
> > > > >>> (TOC)), I have a question:
> > > > >>> Will this responsibility movement (to find appropriate reviewer
> to
> > > > >>> contributor) help us to increase overall throughput?
> > > > >>>
> > > > >>> If we agree constraint in terms of TOC is throughput T2, I
> suggest
> > > > >>> following steps
> > > > >>> - Increase the throughput T2 of the committers
> > > > >>> - Reduce the load on the committers by improve quality of code
> > after
> > > > >> stage
> > > > >>> 1 given to review (pre review by non-committer, automatic review,
> > > code
> > > > >>> inspections)
> > > > >>>
> > > > >>> Best Regards,
> > > > >>> Dmitriy Pavlov
> > > > >>>
> > > > >>>
> > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> > > > >>>
> > > > >>>> Igniters,
> > > > >>>>
> > > > >>>> Currently, according to
> > > > >>>>
> > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > > > >>>> ,
> > > > >>>> contributor can ask for review by moving ticket to PATCH
> AVAILABLE
> > > > >> state.
> > > > >>>>
> > > > >>>> And, as far as I can see, this cause broken tickets issue.
> > > > >>>> Contributor can wait somebody who'll review his changes for a
> > month
> > > or
> > > > >>> even
> > > > >>>> more.
> > > > >>>>
> > > > >>>> I propose to change workflow and *make contributor responsible
> to
> > > find
> > > > >>>> reviewer*.
> > > > >>>> It's pretty easy to find a person able to review changes in most
> > of
> > > > >>> cases.
> > > > >>>>
> > > > >>>> 1) You can check git history of files you modified and find
> > persons
> > > > >> with
> > > > >>>> expertise in this code
> > > > >>>> 2) In case you have problems with such search you can always use
> > > > >>>> maintainers list (
> > > > >>>>
> > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > > > >>>> )
> > > > >>>>
> > > > >>>> Thoughts?
> > > > >>>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Dmitriy Pavlov
Anton,


Thank you for explanation. Personal ask instead of group broadcast may
really help. I understand the idea now.


One argument against solution way 1) it may be not easy for contributor,
especially newcomer, to find a right person.


What do you think about way 2? Personally, I'm ready to help with analysis
and assignment of these 66 tasks from next week.



вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <[hidden email]>:

> Dmitry Pavlov,
>
> There is *HUGE *difference between "Devlist, please review my changes"
> and "Dmitry Pavlov, please review my changes".
>
> In case you're busy right now, you'll, most likely, ignore appeal to
> devlist, but, I'm pretty sure, you'll check appeal to yourself.
> Am I right?
>
> So, my idea is: appeal to devlist is a useless spam maker approach, but
> appeal to person(s) works.
>
> On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
> > Wow, we have 66 tickets waiting for review.... this is pretty bad.
> > Something must definitely change.
> >
> > From my side, having a contributor shop around for a reviewer is not
> really
> > fair. However, I would support the idea of Apache Ignite community
> electing
> > a person responsible to find reviewers for all contributions.
> >
> > D.
> >
> > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <[hidden email]>
> > wrote:
> >
> > > 1) There is waiting for review list here
> > > https://cwiki.apache.org/confluence/display/IGNITE/
> > > Issues+waiting+for+review
> > >
> > > 2) some of contributions are supplemented by dev-list messages (please
> > > review my PR…)
> > >
> > >
> > > And these two tools sometimes do not help. I suppose it is because of
> > > reviewers already have some things to do, but not because of lack of
> tool
> > > support. Do you have other explanations?
> > >
> > >
> > > But still, Igor’s suggestion to notify to dev list sounds reasonable to
> > me.
> > >
> > >
> > >
> > > Anton, could it solve your requirement to know about issue needed to
> > > review?
> > >
> > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
> > >
> > > > By the way, there are emails being sent from Jira to devlist every
> > > > time someone adds comment to ticket, or, for example, edits its
> > > > title which helps a lot to keep a track of ticket's state.
> > > >
> > > > But by some reason there is no such notification when ticket silently
> > > > getting moved to "Patch available" state. I believe, that would help
> if
> > > > there was a notification for that. Is it possible to configure?
> > > >
> > > > Best Regards,
> > > > Igor
> > > >
> > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]>
> wrote:
> > > >
> > > > > In general, I tend to agree with Anton that something needs to be
> > > changed
> > > > > in this direction.
> > > > >
> > > > > How many of you flip through dev list, JIRA or GitHub notifications
> > in
> > > an
> > > > > attempt to find tickets that demand your attention? I bet the
> > > percentage
> > > > is
> > > > > pretty low.
> > > > >
> > > > > To solve this issue I see two options:
> > > > > 1) Proposed by Anton.
> > > > > 2) Having a guy in the community who’ll keep an eye on all the
> > incoming
> > > > > pull-requests shuffling them between committer in the same way
> > proposed
> > > > in
> > > > > 1.
> > > > >
> > > > > Personally, I’m for 1.
> > > > >
> > > > > —
> > > > > Denis
> > > > >
> > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
> [hidden email]>
> > > > > wrote:
> > > > > >
> > > > > > Hi Anton,
> > > > > >
> > > > > >
> > > > > > It is ok for me if it is advice and hint for faster review, as it
> > is
> > > > now.
> > > > > >
> > > > > >
> > > > > > We can periodically remind about this opportunity at dev list or
> in
> > > the
> > > > > > issue comments. We can remind that tasks in patch available
> status
> > > may
> > > > be
> > > > > > reassigned by contributor.
> > > > > >
> > > > > >
> > > > > > Looking from prospective of overall throughput: it is not clear
> for
> > > me
> > > > > how
> > > > > > this process change will help.
> > > > > >
> > > > > >
> > > > > > Best Regards,
> > > > > >
> > > > > > Dmitriy Pavlov
> > > > > >
> > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> > > > > >
> > > > > >> Vova,
> > > > > >>
> > > > > >> Contributors interested to make contributions and I propose them
> > to
> > > > use
> > > > > >> *same* strategy as we (people inside the community) use.
> > > > > >> "-1" will not solve this issue, but my "tips" will.
> > > > > >>
> > > > > >> Dmitry,
> > > > > >>
> > > > > >> The main problem here is that nobody notified that someone is
> > > waiting
> > > > > for
> > > > > >> the review.
> > > > > >> It's not a problem for me to provide tips or to make review, but
> > > it's
> > > > > >> problem to periodically check is there somebody waiting.
> > > > > >>
> > > > > >> Guys,
> > > > > >> Let's try this approach, and I'm pretty sure it will help.
> > > > > >>
> > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> > > [hidden email]>
> > > > > >> wrote:
> > > > > >>
> > > > > >>> Hi Igniters, Anton,
> > > > > >>>
> > > > > >>> Let’s imagine that development process as a chain of production
> > > > stages
> > > > > >>> 1) Developing patch by contributor
> > > > > >>> 2) Review changes by committer
> > > > > >>>
> > > > > >>> Reviews waiting too long time to be done at stage 2 may
> indicate
> > > that
> > > > > >> speed
> > > > > >>> (potential throughput) of step 2 is less than throughput at
> step
> > 1.
> > > > > T2<T1
> > > > > >>>
> > > > > >>> In terms of this model (inspired by Goldratt’s Theory of
> > > Constraints
> > > > > >>> (TOC)), I have a question:
> > > > > >>> Will this responsibility movement (to find appropriate reviewer
> > to
> > > > > >>> contributor) help us to increase overall throughput?
> > > > > >>>
> > > > > >>> If we agree constraint in terms of TOC is throughput T2, I
> > suggest
> > > > > >>> following steps
> > > > > >>> - Increase the throughput T2 of the committers
> > > > > >>> - Reduce the load on the committers by improve quality of code
> > > after
> > > > > >> stage
> > > > > >>> 1 given to review (pre review by non-committer, automatic
> review,
> > > > code
> > > > > >>> inspections)
> > > > > >>>
> > > > > >>> Best Regards,
> > > > > >>> Dmitriy Pavlov
> > > > > >>>
> > > > > >>>
> > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
> > > > > >>>
> > > > > >>>> Igniters,
> > > > > >>>>
> > > > > >>>> Currently, according to
> > > > > >>>>
> > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > > > > >>>> ,
> > > > > >>>> contributor can ask for review by moving ticket to PATCH
> > AVAILABLE
> > > > > >> state.
> > > > > >>>>
> > > > > >>>> And, as far as I can see, this cause broken tickets issue.
> > > > > >>>> Contributor can wait somebody who'll review his changes for a
> > > month
> > > > or
> > > > > >>> even
> > > > > >>>> more.
> > > > > >>>>
> > > > > >>>> I propose to change workflow and *make contributor responsible
> > to
> > > > find
> > > > > >>>> reviewer*.
> > > > > >>>> It's pretty easy to find a person able to review changes in
> most
> > > of
> > > > > >>> cases.
> > > > > >>>>
> > > > > >>>> 1) You can check git history of files you modified and find
> > > persons
> > > > > >> with
> > > > > >>>> expertise in this code
> > > > > >>>> 2) In case you have problems with such search you can always
> use
> > > > > >>>> maintainers list (
> > > > > >>>>
> > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > > > > >>>> )
> > > > > >>>>
> > > > > >>>> Thoughts?
> > > > > >>>>
> > > > > >>>
> > > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov
Dmitry,

1) See my initial email, it contains instruction how to find a reviewer.
And it's pretty easy to do when you have something to review (you did some
code changes).

I want to add following to our wiki:

"
Ask commiter to review changes.
Check affected file's git history to find commiter most likely able to
review changes.
In case it's hard to determine who's able to review by git history use
maintainers list presented above.
Add "review request" comment to the ticket starting with a commiter
username.

for example: "[~avinogradov], Please review my changes."

Commiter will gain notification and review your changes and/or find another
commiter to do this.

Important: Each comment should be started with [~username].
"

2) It will be a huge help to the community!

On Tue, Jun 6, 2017 at 1:12 PM, Dmitry Pavlov <[hidden email]> wrote:

> Anton,
>
>
> Thank you for explanation. Personal ask instead of group broadcast may
> really help. I understand the idea now.
>
>
> One argument against solution way 1) it may be not easy for contributor,
> especially newcomer, to find a right person.
>
>
> What do you think about way 2? Personally, I'm ready to help with analysis
> and assignment of these 66 tasks from next week.
>
>
>
> вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <[hidden email]>:
>
> > Dmitry Pavlov,
> >
> > There is *HUGE *difference between "Devlist, please review my changes"
> > and "Dmitry Pavlov, please review my changes".
> >
> > In case you're busy right now, you'll, most likely, ignore appeal to
> > devlist, but, I'm pretty sure, you'll check appeal to yourself.
> > Am I right?
> >
> > So, my idea is: appeal to devlist is a useless spam maker approach, but
> > appeal to person(s) works.
> >
> > On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <[hidden email]
> >
> > wrote:
> >
> > > Wow, we have 66 tickets waiting for review.... this is pretty bad.
> > > Something must definitely change.
> > >
> > > From my side, having a contributor shop around for a reviewer is not
> > really
> > > fair. However, I would support the idea of Apache Ignite community
> > electing
> > > a person responsible to find reviewers for all contributions.
> > >
> > > D.
> > >
> > > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <[hidden email]>
> > > wrote:
> > >
> > > > 1) There is waiting for review list here
> > > > https://cwiki.apache.org/confluence/display/IGNITE/
> > > > Issues+waiting+for+review
> > > >
> > > > 2) some of contributions are supplemented by dev-list messages
> (please
> > > > review my PR…)
> > > >
> > > >
> > > > And these two tools sometimes do not help. I suppose it is because of
> > > > reviewers already have some things to do, but not because of lack of
> > tool
> > > > support. Do you have other explanations?
> > > >
> > > >
> > > > But still, Igor’s suggestion to notify to dev list sounds reasonable
> to
> > > me.
> > > >
> > > >
> > > >
> > > > Anton, could it solve your requirement to know about issue needed to
> > > > review?
> > > >
> > > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
> > > >
> > > > > By the way, there are emails being sent from Jira to devlist every
> > > > > time someone adds comment to ticket, or, for example, edits its
> > > > > title which helps a lot to keep a track of ticket's state.
> > > > >
> > > > > But by some reason there is no such notification when ticket
> silently
> > > > > getting moved to "Patch available" state. I believe, that would
> help
> > if
> > > > > there was a notification for that. Is it possible to configure?
> > > > >
> > > > > Best Regards,
> > > > > Igor
> > > > >
> > > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]>
> > wrote:
> > > > >
> > > > > > In general, I tend to agree with Anton that something needs to be
> > > > changed
> > > > > > in this direction.
> > > > > >
> > > > > > How many of you flip through dev list, JIRA or GitHub
> notifications
> > > in
> > > > an
> > > > > > attempt to find tickets that demand your attention? I bet the
> > > > percentage
> > > > > is
> > > > > > pretty low.
> > > > > >
> > > > > > To solve this issue I see two options:
> > > > > > 1) Proposed by Anton.
> > > > > > 2) Having a guy in the community who’ll keep an eye on all the
> > > incoming
> > > > > > pull-requests shuffling them between committer in the same way
> > > proposed
> > > > > in
> > > > > > 1.
> > > > > >
> > > > > > Personally, I’m for 1.
> > > > > >
> > > > > > —
> > > > > > Denis
> > > > > >
> > > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
> > [hidden email]>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Anton,
> > > > > > >
> > > > > > >
> > > > > > > It is ok for me if it is advice and hint for faster review, as
> it
> > > is
> > > > > now.
> > > > > > >
> > > > > > >
> > > > > > > We can periodically remind about this opportunity at dev list
> or
> > in
> > > > the
> > > > > > > issue comments. We can remind that tasks in patch available
> > status
> > > > may
> > > > > be
> > > > > > > reassigned by contributor.
> > > > > > >
> > > > > > >
> > > > > > > Looking from prospective of overall throughput: it is not clear
> > for
> > > > me
> > > > > > how
> > > > > > > this process change will help.
> > > > > > >
> > > > > > >
> > > > > > > Best Regards,
> > > > > > >
> > > > > > > Dmitriy Pavlov
> > > > > > >
> > > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
> > > > > > >
> > > > > > >> Vova,
> > > > > > >>
> > > > > > >> Contributors interested to make contributions and I propose
> them
> > > to
> > > > > use
> > > > > > >> *same* strategy as we (people inside the community) use.
> > > > > > >> "-1" will not solve this issue, but my "tips" will.
> > > > > > >>
> > > > > > >> Dmitry,
> > > > > > >>
> > > > > > >> The main problem here is that nobody notified that someone is
> > > > waiting
> > > > > > for
> > > > > > >> the review.
> > > > > > >> It's not a problem for me to provide tips or to make review,
> but
> > > > it's
> > > > > > >> problem to periodically check is there somebody waiting.
> > > > > > >>
> > > > > > >> Guys,
> > > > > > >> Let's try this approach, and I'm pretty sure it will help.
> > > > > > >>
> > > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> > > > [hidden email]>
> > > > > > >> wrote:
> > > > > > >>
> > > > > > >>> Hi Igniters, Anton,
> > > > > > >>>
> > > > > > >>> Let’s imagine that development process as a chain of
> production
> > > > > stages
> > > > > > >>> 1) Developing patch by contributor
> > > > > > >>> 2) Review changes by committer
> > > > > > >>>
> > > > > > >>> Reviews waiting too long time to be done at stage 2 may
> > indicate
> > > > that
> > > > > > >> speed
> > > > > > >>> (potential throughput) of step 2 is less than throughput at
> > step
> > > 1.
> > > > > > T2<T1
> > > > > > >>>
> > > > > > >>> In terms of this model (inspired by Goldratt’s Theory of
> > > > Constraints
> > > > > > >>> (TOC)), I have a question:
> > > > > > >>> Will this responsibility movement (to find appropriate
> reviewer
> > > to
> > > > > > >>> contributor) help us to increase overall throughput?
> > > > > > >>>
> > > > > > >>> If we agree constraint in terms of TOC is throughput T2, I
> > > suggest
> > > > > > >>> following steps
> > > > > > >>> - Increase the throughput T2 of the committers
> > > > > > >>> - Reduce the load on the committers by improve quality of
> code
> > > > after
> > > > > > >> stage
> > > > > > >>> 1 given to review (pre review by non-committer, automatic
> > review,
> > > > > code
> > > > > > >>> inspections)
> > > > > > >>>
> > > > > > >>> Best Regards,
> > > > > > >>> Dmitriy Pavlov
> > > > > > >>>
> > > > > > >>>
> > > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]
> >:
> > > > > > >>>
> > > > > > >>>> Igniters,
> > > > > > >>>>
> > > > > > >>>> Currently, according to
> > > > > > >>>>
> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > > > > > >>>> ,
> > > > > > >>>> contributor can ask for review by moving ticket to PATCH
> > > AVAILABLE
> > > > > > >> state.
> > > > > > >>>>
> > > > > > >>>> And, as far as I can see, this cause broken tickets issue.
> > > > > > >>>> Contributor can wait somebody who'll review his changes for
> a
> > > > month
> > > > > or
> > > > > > >>> even
> > > > > > >>>> more.
> > > > > > >>>>
> > > > > > >>>> I propose to change workflow and *make contributor
> responsible
> > > to
> > > > > find
> > > > > > >>>> reviewer*.
> > > > > > >>>> It's pretty easy to find a person able to review changes in
> > most
> > > > of
> > > > > > >>> cases.
> > > > > > >>>>
> > > > > > >>>> 1) You can check git history of files you modified and find
> > > > persons
> > > > > > >> with
> > > > > > >>>> expertise in this code
> > > > > > >>>> 2) In case you have problems with such search you can always
> > use
> > > > > > >>>> maintainers list (
> > > > > > >>>>
> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> > > > > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > > > > > >>>> )
> > > > > > >>>>
> > > > > > >>>> Thoughts?
> > > > > > >>>>
> > > > > > >>>
> > > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov
Igniters,

Since we found that proposed approach can help,
no one mind that I'll add text listed above to the wiki?

On Tue, Jun 6, 2017 at 1:19 PM, Anton Vinogradov <[hidden email]>
wrote:

> Dmitry,
>
> 1) See my initial email, it contains instruction how to find a reviewer.
> And it's pretty easy to do when you have something to review (you did some
> code changes).
>
> I want to add following to our wiki:
>
> "
> Ask commiter to review changes.
> Check affected file's git history to find commiter most likely able to
> review changes.
> In case it's hard to determine who's able to review by git history use
> maintainers list presented above.
> Add "review request" comment to the ticket starting with a commiter
> username.
>
> for example: "[~avinogradov], Please review my changes."
>
> Commiter will gain notification and review your changes and/or find
> another commiter to do this.
>
> Important: Each comment should be started with [~username].
> "
>
> 2) It will be a huge help to the community!
>
> On Tue, Jun 6, 2017 at 1:12 PM, Dmitry Pavlov <[hidden email]>
> wrote:
>
>> Anton,
>>
>>
>> Thank you for explanation. Personal ask instead of group broadcast may
>> really help. I understand the idea now.
>>
>>
>> One argument against solution way 1) it may be not easy for contributor,
>> especially newcomer, to find a right person.
>>
>>
>> What do you think about way 2? Personally, I'm ready to help with analysis
>> and assignment of these 66 tasks from next week.
>>
>>
>>
>> вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <[hidden email]>:
>>
>> > Dmitry Pavlov,
>> >
>> > There is *HUGE *difference between "Devlist, please review my changes"
>> > and "Dmitry Pavlov, please review my changes".
>> >
>> > In case you're busy right now, you'll, most likely, ignore appeal to
>> > devlist, but, I'm pretty sure, you'll check appeal to yourself.
>> > Am I right?
>> >
>> > So, my idea is: appeal to devlist is a useless spam maker approach, but
>> > appeal to person(s) works.
>> >
>> > On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <
>> [hidden email]>
>> > wrote:
>> >
>> > > Wow, we have 66 tickets waiting for review.... this is pretty bad.
>> > > Something must definitely change.
>> > >
>> > > From my side, having a contributor shop around for a reviewer is not
>> > really
>> > > fair. However, I would support the idea of Apache Ignite community
>> > electing
>> > > a person responsible to find reviewers for all contributions.
>> > >
>> > > D.
>> > >
>> > > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <[hidden email]
>> >
>> > > wrote:
>> > >
>> > > > 1) There is waiting for review list here
>> > > > https://cwiki.apache.org/confluence/display/IGNITE/
>> > > > Issues+waiting+for+review
>> > > >
>> > > > 2) some of contributions are supplemented by dev-list messages
>> (please
>> > > > review my PR…)
>> > > >
>> > > >
>> > > > And these two tools sometimes do not help. I suppose it is because
>> of
>> > > > reviewers already have some things to do, but not because of lack of
>> > tool
>> > > > support. Do you have other explanations?
>> > > >
>> > > >
>> > > > But still, Igor’s suggestion to notify to dev list sounds
>> reasonable to
>> > > me.
>> > > >
>> > > >
>> > > >
>> > > > Anton, could it solve your requirement to know about issue needed to
>> > > > review?
>> > > >
>> > > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
>> > > >
>> > > > > By the way, there are emails being sent from Jira to devlist every
>> > > > > time someone adds comment to ticket, or, for example, edits its
>> > > > > title which helps a lot to keep a track of ticket's state.
>> > > > >
>> > > > > But by some reason there is no such notification when ticket
>> silently
>> > > > > getting moved to "Patch available" state. I believe, that would
>> help
>> > if
>> > > > > there was a notification for that. Is it possible to configure?
>> > > > >
>> > > > > Best Regards,
>> > > > > Igor
>> > > > >
>> > > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]>
>> > wrote:
>> > > > >
>> > > > > > In general, I tend to agree with Anton that something needs to
>> be
>> > > > changed
>> > > > > > in this direction.
>> > > > > >
>> > > > > > How many of you flip through dev list, JIRA or GitHub
>> notifications
>> > > in
>> > > > an
>> > > > > > attempt to find tickets that demand your attention? I bet the
>> > > > percentage
>> > > > > is
>> > > > > > pretty low.
>> > > > > >
>> > > > > > To solve this issue I see two options:
>> > > > > > 1) Proposed by Anton.
>> > > > > > 2) Having a guy in the community who’ll keep an eye on all the
>> > > incoming
>> > > > > > pull-requests shuffling them between committer in the same way
>> > > proposed
>> > > > > in
>> > > > > > 1.
>> > > > > >
>> > > > > > Personally, I’m for 1.
>> > > > > >
>> > > > > > —
>> > > > > > Denis
>> > > > > >
>> > > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
>> > [hidden email]>
>> > > > > > wrote:
>> > > > > > >
>> > > > > > > Hi Anton,
>> > > > > > >
>> > > > > > >
>> > > > > > > It is ok for me if it is advice and hint for faster review,
>> as it
>> > > is
>> > > > > now.
>> > > > > > >
>> > > > > > >
>> > > > > > > We can periodically remind about this opportunity at dev list
>> or
>> > in
>> > > > the
>> > > > > > > issue comments. We can remind that tasks in patch available
>> > status
>> > > > may
>> > > > > be
>> > > > > > > reassigned by contributor.
>> > > > > > >
>> > > > > > >
>> > > > > > > Looking from prospective of overall throughput: it is not
>> clear
>> > for
>> > > > me
>> > > > > > how
>> > > > > > > this process change will help.
>> > > > > > >
>> > > > > > >
>> > > > > > > Best Regards,
>> > > > > > >
>> > > > > > > Dmitriy Pavlov
>> > > > > > >
>> > > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]>:
>> > > > > > >
>> > > > > > >> Vova,
>> > > > > > >>
>> > > > > > >> Contributors interested to make contributions and I propose
>> them
>> > > to
>> > > > > use
>> > > > > > >> *same* strategy as we (people inside the community) use.
>> > > > > > >> "-1" will not solve this issue, but my "tips" will.
>> > > > > > >>
>> > > > > > >> Dmitry,
>> > > > > > >>
>> > > > > > >> The main problem here is that nobody notified that someone is
>> > > > waiting
>> > > > > > for
>> > > > > > >> the review.
>> > > > > > >> It's not a problem for me to provide tips or to make review,
>> but
>> > > > it's
>> > > > > > >> problem to periodically check is there somebody waiting.
>> > > > > > >>
>> > > > > > >> Guys,
>> > > > > > >> Let's try this approach, and I'm pretty sure it will help.
>> > > > > > >>
>> > > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
>> > > > [hidden email]>
>> > > > > > >> wrote:
>> > > > > > >>
>> > > > > > >>> Hi Igniters, Anton,
>> > > > > > >>>
>> > > > > > >>> Let’s imagine that development process as a chain of
>> production
>> > > > > stages
>> > > > > > >>> 1) Developing patch by contributor
>> > > > > > >>> 2) Review changes by committer
>> > > > > > >>>
>> > > > > > >>> Reviews waiting too long time to be done at stage 2 may
>> > indicate
>> > > > that
>> > > > > > >> speed
>> > > > > > >>> (potential throughput) of step 2 is less than throughput at
>> > step
>> > > 1.
>> > > > > > T2<T1
>> > > > > > >>>
>> > > > > > >>> In terms of this model (inspired by Goldratt’s Theory of
>> > > > Constraints
>> > > > > > >>> (TOC)), I have a question:
>> > > > > > >>> Will this responsibility movement (to find appropriate
>> reviewer
>> > > to
>> > > > > > >>> contributor) help us to increase overall throughput?
>> > > > > > >>>
>> > > > > > >>> If we agree constraint in terms of TOC is throughput T2, I
>> > > suggest
>> > > > > > >>> following steps
>> > > > > > >>> - Increase the throughput T2 of the committers
>> > > > > > >>> - Reduce the load on the committers by improve quality of
>> code
>> > > > after
>> > > > > > >> stage
>> > > > > > >>> 1 given to review (pre review by non-committer, automatic
>> > review,
>> > > > > code
>> > > > > > >>> inspections)
>> > > > > > >>>
>> > > > > > >>> Best Regards,
>> > > > > > >>> Dmitriy Pavlov
>> > > > > > >>>
>> > > > > > >>>
>> > > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]
>> >:
>> > > > > > >>>
>> > > > > > >>>> Igniters,
>> > > > > > >>>>
>> > > > > > >>>> Currently, according to
>> > > > > > >>>>
>> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> > > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
>> > > > > > >>>> ,
>> > > > > > >>>> contributor can ask for review by moving ticket to PATCH
>> > > AVAILABLE
>> > > > > > >> state.
>> > > > > > >>>>
>> > > > > > >>>> And, as far as I can see, this cause broken tickets issue.
>> > > > > > >>>> Contributor can wait somebody who'll review his changes
>> for a
>> > > > month
>> > > > > or
>> > > > > > >>> even
>> > > > > > >>>> more.
>> > > > > > >>>>
>> > > > > > >>>> I propose to change workflow and *make contributor
>> responsible
>> > > to
>> > > > > find
>> > > > > > >>>> reviewer*.
>> > > > > > >>>> It's pretty easy to find a person able to review changes in
>> > most
>> > > > of
>> > > > > > >>> cases.
>> > > > > > >>>>
>> > > > > > >>>> 1) You can check git history of files you modified and find
>> > > > persons
>> > > > > > >> with
>> > > > > > >>>> expertise in this code
>> > > > > > >>>> 2) In case you have problems with such search you can
>> always
>> > use
>> > > > > > >>>> maintainers list (
>> > > > > > >>>>
>> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
>> > > > > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
>> > > > > > >>>> )
>> > > > > > >>>>
>> > > > > > >>>> Thoughts?
>> > > > > > >>>>
>> > > > > > >>>
>> > > > > > >>
>> > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

dsetrakyan
On Tue, Jun 6, 2017 at 7:40 AM, Anton Vinogradov <[hidden email]>
wrote:

> Igniters,
>
> Since we found that proposed approach can help,
> no one mind that I'll add text listed above to the wiki?
>

I don't think that we have an agreement yet. Again, I still don't think it
is fair for a contributor to find a committer that has a relevant area of
expertise. A contributor should feel free to ask any committer for a
review, but it should not be mandatory. I would rather have an existing
contributor or committer help with finding a reviewer.


>
> On Tue, Jun 6, 2017 at 1:19 PM, Anton Vinogradov <[hidden email]
> >
> wrote:
>
> > Dmitry,
> >
> > 1) See my initial email, it contains instruction how to find a reviewer.
> > And it's pretty easy to do when you have something to review (you did
> some
> > code changes).
> >
> > I want to add following to our wiki:
> >
> > "
> > Ask commiter to review changes.
> > Check affected file's git history to find commiter most likely able to
> > review changes.
> > In case it's hard to determine who's able to review by git history use
> > maintainers list presented above.
> > Add "review request" comment to the ticket starting with a commiter
> > username.
> >
> > for example: "[~avinogradov], Please review my changes."
> >
> > Commiter will gain notification and review your changes and/or find
> > another commiter to do this.
> >
> > Important: Each comment should be started with [~username].
> > "
> >
> > 2) It will be a huge help to the community!
> >
> > On Tue, Jun 6, 2017 at 1:12 PM, Dmitry Pavlov <[hidden email]>
> > wrote:
> >
> >> Anton,
> >>
> >>
> >> Thank you for explanation. Personal ask instead of group broadcast may
> >> really help. I understand the idea now.
> >>
> >>
> >> One argument against solution way 1) it may be not easy for contributor,
> >> especially newcomer, to find a right person.
> >>
> >>
> >> What do you think about way 2? Personally, I'm ready to help with
> analysis
> >> and assignment of these 66 tasks from next week.
> >>
> >>
> >>
> >> вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <[hidden email]
> >:
> >>
> >> > Dmitry Pavlov,
> >> >
> >> > There is *HUGE *difference between "Devlist, please review my changes"
> >> > and "Dmitry Pavlov, please review my changes".
> >> >
> >> > In case you're busy right now, you'll, most likely, ignore appeal to
> >> > devlist, but, I'm pretty sure, you'll check appeal to yourself.
> >> > Am I right?
> >> >
> >> > So, my idea is: appeal to devlist is a useless spam maker approach,
> but
> >> > appeal to person(s) works.
> >> >
> >> > On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <
> >> [hidden email]>
> >> > wrote:
> >> >
> >> > > Wow, we have 66 tickets waiting for review.... this is pretty bad.
> >> > > Something must definitely change.
> >> > >
> >> > > From my side, having a contributor shop around for a reviewer is not
> >> > really
> >> > > fair. However, I would support the idea of Apache Ignite community
> >> > electing
> >> > > a person responsible to find reviewers for all contributions.
> >> > >
> >> > > D.
> >> > >
> >> > > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <
> [hidden email]
> >> >
> >> > > wrote:
> >> > >
> >> > > > 1) There is waiting for review list here
> >> > > > https://cwiki.apache.org/confluence/display/IGNITE/
> >> > > > Issues+waiting+for+review
> >> > > >
> >> > > > 2) some of contributions are supplemented by dev-list messages
> >> (please
> >> > > > review my PR…)
> >> > > >
> >> > > >
> >> > > > And these two tools sometimes do not help. I suppose it is because
> >> of
> >> > > > reviewers already have some things to do, but not because of lack
> of
> >> > tool
> >> > > > support. Do you have other explanations?
> >> > > >
> >> > > >
> >> > > > But still, Igor’s suggestion to notify to dev list sounds
> >> reasonable to
> >> > > me.
> >> > > >
> >> > > >
> >> > > >
> >> > > > Anton, could it solve your requirement to know about issue needed
> to
> >> > > > review?
> >> > > >
> >> > > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
> >> > > >
> >> > > > > By the way, there are emails being sent from Jira to devlist
> every
> >> > > > > time someone adds comment to ticket, or, for example, edits its
> >> > > > > title which helps a lot to keep a track of ticket's state.
> >> > > > >
> >> > > > > But by some reason there is no such notification when ticket
> >> silently
> >> > > > > getting moved to "Patch available" state. I believe, that would
> >> help
> >> > if
> >> > > > > there was a notification for that. Is it possible to configure?
> >> > > > >
> >> > > > > Best Regards,
> >> > > > > Igor
> >> > > > >
> >> > > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <[hidden email]>
> >> > wrote:
> >> > > > >
> >> > > > > > In general, I tend to agree with Anton that something needs to
> >> be
> >> > > > changed
> >> > > > > > in this direction.
> >> > > > > >
> >> > > > > > How many of you flip through dev list, JIRA or GitHub
> >> notifications
> >> > > in
> >> > > > an
> >> > > > > > attempt to find tickets that demand your attention? I bet the
> >> > > > percentage
> >> > > > > is
> >> > > > > > pretty low.
> >> > > > > >
> >> > > > > > To solve this issue I see two options:
> >> > > > > > 1) Proposed by Anton.
> >> > > > > > 2) Having a guy in the community who’ll keep an eye on all the
> >> > > incoming
> >> > > > > > pull-requests shuffling them between committer in the same way
> >> > > proposed
> >> > > > > in
> >> > > > > > 1.
> >> > > > > >
> >> > > > > > Personally, I’m for 1.
> >> > > > > >
> >> > > > > > —
> >> > > > > > Denis
> >> > > > > >
> >> > > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
> >> > [hidden email]>
> >> > > > > > wrote:
> >> > > > > > >
> >> > > > > > > Hi Anton,
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > It is ok for me if it is advice and hint for faster review,
> >> as it
> >> > > is
> >> > > > > now.
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > We can periodically remind about this opportunity at dev
> list
> >> or
> >> > in
> >> > > > the
> >> > > > > > > issue comments. We can remind that tasks in patch available
> >> > status
> >> > > > may
> >> > > > > be
> >> > > > > > > reassigned by contributor.
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > Looking from prospective of overall throughput: it is not
> >> clear
> >> > for
> >> > > > me
> >> > > > > > how
> >> > > > > > > this process change will help.
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > Best Regards,
> >> > > > > > >
> >> > > > > > > Dmitriy Pavlov
> >> > > > > > >
> >> > > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <[hidden email]
> >:
> >> > > > > > >
> >> > > > > > >> Vova,
> >> > > > > > >>
> >> > > > > > >> Contributors interested to make contributions and I propose
> >> them
> >> > > to
> >> > > > > use
> >> > > > > > >> *same* strategy as we (people inside the community) use.
> >> > > > > > >> "-1" will not solve this issue, but my "tips" will.
> >> > > > > > >>
> >> > > > > > >> Dmitry,
> >> > > > > > >>
> >> > > > > > >> The main problem here is that nobody notified that someone
> is
> >> > > > waiting
> >> > > > > > for
> >> > > > > > >> the review.
> >> > > > > > >> It's not a problem for me to provide tips or to make
> review,
> >> but
> >> > > > it's
> >> > > > > > >> problem to periodically check is there somebody waiting.
> >> > > > > > >>
> >> > > > > > >> Guys,
> >> > > > > > >> Let's try this approach, and I'm pretty sure it will help.
> >> > > > > > >>
> >> > > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> >> > > > [hidden email]>
> >> > > > > > >> wrote:
> >> > > > > > >>
> >> > > > > > >>> Hi Igniters, Anton,
> >> > > > > > >>>
> >> > > > > > >>> Let’s imagine that development process as a chain of
> >> production
> >> > > > > stages
> >> > > > > > >>> 1) Developing patch by contributor
> >> > > > > > >>> 2) Review changes by committer
> >> > > > > > >>>
> >> > > > > > >>> Reviews waiting too long time to be done at stage 2 may
> >> > indicate
> >> > > > that
> >> > > > > > >> speed
> >> > > > > > >>> (potential throughput) of step 2 is less than throughput
> at
> >> > step
> >> > > 1.
> >> > > > > > T2<T1
> >> > > > > > >>>
> >> > > > > > >>> In terms of this model (inspired by Goldratt’s Theory of
> >> > > > Constraints
> >> > > > > > >>> (TOC)), I have a question:
> >> > > > > > >>> Will this responsibility movement (to find appropriate
> >> reviewer
> >> > > to
> >> > > > > > >>> contributor) help us to increase overall throughput?
> >> > > > > > >>>
> >> > > > > > >>> If we agree constraint in terms of TOC is throughput T2, I
> >> > > suggest
> >> > > > > > >>> following steps
> >> > > > > > >>> - Increase the throughput T2 of the committers
> >> > > > > > >>> - Reduce the load on the committers by improve quality of
> >> code
> >> > > > after
> >> > > > > > >> stage
> >> > > > > > >>> 1 given to review (pre review by non-committer, automatic
> >> > review,
> >> > > > > code
> >> > > > > > >>> inspections)
> >> > > > > > >>>
> >> > > > > > >>> Best Regards,
> >> > > > > > >>> Dmitriy Pavlov
> >> > > > > > >>>
> >> > > > > > >>>
> >> > > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <
> [hidden email]
> >> >:
> >> > > > > > >>>
> >> > > > > > >>>> Igniters,
> >> > > > > > >>>>
> >> > > > > > >>>> Currently, according to
> >> > > > > > >>>>
> >> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >> > > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> >> > > > > > >>>> ,
> >> > > > > > >>>> contributor can ask for review by moving ticket to PATCH
> >> > > AVAILABLE
> >> > > > > > >> state.
> >> > > > > > >>>>
> >> > > > > > >>>> And, as far as I can see, this cause broken tickets
> issue.
> >> > > > > > >>>> Contributor can wait somebody who'll review his changes
> >> for a
> >> > > > month
> >> > > > > or
> >> > > > > > >>> even
> >> > > > > > >>>> more.
> >> > > > > > >>>>
> >> > > > > > >>>> I propose to change workflow and *make contributor
> >> responsible
> >> > > to
> >> > > > > find
> >> > > > > > >>>> reviewer*.
> >> > > > > > >>>> It's pretty easy to find a person able to review changes
> in
> >> > most
> >> > > > of
> >> > > > > > >>> cases.
> >> > > > > > >>>>
> >> > > > > > >>>> 1) You can check git history of files you modified and
> find
> >> > > > persons
> >> > > > > > >> with
> >> > > > > > >>>> expertise in this code
> >> > > > > > >>>> 2) In case you have problems with such search you can
> >> always
> >> > use
> >> > > > > > >>>> maintainers list (
> >> > > > > > >>>>
> >> > > > > > >>>> https://cwiki.apache.org/confluence/display/IGNITE/How+
> >> > > > > > >>> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> >> > > > > > >>>> )
> >> > > > > > >>>>
> >> > > > > > >>>> Thoughts?
> >> > > > > > >>>>
> >> > > > > > >>>
> >> > > > > > >>
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov
Dmitriy,

Got it,
I'll add this as an optional "Tips to pass review quickly".

On Tue, Jun 6, 2017 at 7:11 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> On Tue, Jun 6, 2017 at 7:40 AM, Anton Vinogradov <[hidden email]
> >
> wrote:
>
> > Igniters,
> >
> > Since we found that proposed approach can help,
> > no one mind that I'll add text listed above to the wiki?
> >
>
> I don't think that we have an agreement yet. Again, I still don't think it
> is fair for a contributor to find a committer that has a relevant area of
> expertise. A contributor should feel free to ask any committer for a
> review, but it should not be mandatory. I would rather have an existing
> contributor or committer help with finding a reviewer.
>
>
> >
> > On Tue, Jun 6, 2017 at 1:19 PM, Anton Vinogradov <
> [hidden email]
> > >
> > wrote:
> >
> > > Dmitry,
> > >
> > > 1) See my initial email, it contains instruction how to find a
> reviewer.
> > > And it's pretty easy to do when you have something to review (you did
> > some
> > > code changes).
> > >
> > > I want to add following to our wiki:
> > >
> > > "
> > > Ask commiter to review changes.
> > > Check affected file's git history to find commiter most likely able to
> > > review changes.
> > > In case it's hard to determine who's able to review by git history use
> > > maintainers list presented above.
> > > Add "review request" comment to the ticket starting with a commiter
> > > username.
> > >
> > > for example: "[~avinogradov], Please review my changes."
> > >
> > > Commiter will gain notification and review your changes and/or find
> > > another commiter to do this.
> > >
> > > Important: Each comment should be started with [~username].
> > > "
> > >
> > > 2) It will be a huge help to the community!
> > >
> > > On Tue, Jun 6, 2017 at 1:12 PM, Dmitry Pavlov <[hidden email]>
> > > wrote:
> > >
> > >> Anton,
> > >>
> > >>
> > >> Thank you for explanation. Personal ask instead of group broadcast may
> > >> really help. I understand the idea now.
> > >>
> > >>
> > >> One argument against solution way 1) it may be not easy for
> contributor,
> > >> especially newcomer, to find a right person.
> > >>
> > >>
> > >> What do you think about way 2? Personally, I'm ready to help with
> > analysis
> > >> and assignment of these 66 tasks from next week.
> > >>
> > >>
> > >>
> > >> вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <
> [hidden email]
> > >:
> > >>
> > >> > Dmitry Pavlov,
> > >> >
> > >> > There is *HUGE *difference between "Devlist, please review my
> changes"
> > >> > and "Dmitry Pavlov, please review my changes".
> > >> >
> > >> > In case you're busy right now, you'll, most likely, ignore appeal to
> > >> > devlist, but, I'm pretty sure, you'll check appeal to yourself.
> > >> > Am I right?
> > >> >
> > >> > So, my idea is: appeal to devlist is a useless spam maker approach,
> > but
> > >> > appeal to person(s) works.
> > >> >
> > >> > On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <
> > >> [hidden email]>
> > >> > wrote:
> > >> >
> > >> > > Wow, we have 66 tickets waiting for review.... this is pretty bad.
> > >> > > Something must definitely change.
> > >> > >
> > >> > > From my side, having a contributor shop around for a reviewer is
> not
> > >> > really
> > >> > > fair. However, I would support the idea of Apache Ignite community
> > >> > electing
> > >> > > a person responsible to find reviewers for all contributions.
> > >> > >
> > >> > > D.
> > >> > >
> > >> > > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <
> > [hidden email]
> > >> >
> > >> > > wrote:
> > >> > >
> > >> > > > 1) There is waiting for review list here
> > >> > > > https://cwiki.apache.org/confluence/display/IGNITE/
> > >> > > > Issues+waiting+for+review
> > >> > > >
> > >> > > > 2) some of contributions are supplemented by dev-list messages
> > >> (please
> > >> > > > review my PR…)
> > >> > > >
> > >> > > >
> > >> > > > And these two tools sometimes do not help. I suppose it is
> because
> > >> of
> > >> > > > reviewers already have some things to do, but not because of
> lack
> > of
> > >> > tool
> > >> > > > support. Do you have other explanations?
> > >> > > >
> > >> > > >
> > >> > > > But still, Igor’s suggestion to notify to dev list sounds
> > >> reasonable to
> > >> > > me.
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > > Anton, could it solve your requirement to know about issue
> needed
> > to
> > >> > > > review?
> > >> > > >
> > >> > > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]>:
> > >> > > >
> > >> > > > > By the way, there are emails being sent from Jira to devlist
> > every
> > >> > > > > time someone adds comment to ticket, or, for example, edits
> its
> > >> > > > > title which helps a lot to keep a track of ticket's state.
> > >> > > > >
> > >> > > > > But by some reason there is no such notification when ticket
> > >> silently
> > >> > > > > getting moved to "Patch available" state. I believe, that
> would
> > >> help
> > >> > if
> > >> > > > > there was a notification for that. Is it possible to
> configure?
> > >> > > > >
> > >> > > > > Best Regards,
> > >> > > > > Igor
> > >> > > > >
> > >> > > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <
> [hidden email]>
> > >> > wrote:
> > >> > > > >
> > >> > > > > > In general, I tend to agree with Anton that something needs
> to
> > >> be
> > >> > > > changed
> > >> > > > > > in this direction.
> > >> > > > > >
> > >> > > > > > How many of you flip through dev list, JIRA or GitHub
> > >> notifications
> > >> > > in
> > >> > > > an
> > >> > > > > > attempt to find tickets that demand your attention? I bet
> the
> > >> > > > percentage
> > >> > > > > is
> > >> > > > > > pretty low.
> > >> > > > > >
> > >> > > > > > To solve this issue I see two options:
> > >> > > > > > 1) Proposed by Anton.
> > >> > > > > > 2) Having a guy in the community who’ll keep an eye on all
> the
> > >> > > incoming
> > >> > > > > > pull-requests shuffling them between committer in the same
> way
> > >> > > proposed
> > >> > > > > in
> > >> > > > > > 1.
> > >> > > > > >
> > >> > > > > > Personally, I’m for 1.
> > >> > > > > >
> > >> > > > > > —
> > >> > > > > > Denis
> > >> > > > > >
> > >> > > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
> > >> > [hidden email]>
> > >> > > > > > wrote:
> > >> > > > > > >
> > >> > > > > > > Hi Anton,
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > It is ok for me if it is advice and hint for faster
> review,
> > >> as it
> > >> > > is
> > >> > > > > now.
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > We can periodically remind about this opportunity at dev
> > list
> > >> or
> > >> > in
> > >> > > > the
> > >> > > > > > > issue comments. We can remind that tasks in patch
> available
> > >> > status
> > >> > > > may
> > >> > > > > be
> > >> > > > > > > reassigned by contributor.
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > Looking from prospective of overall throughput: it is not
> > >> clear
> > >> > for
> > >> > > > me
> > >> > > > > > how
> > >> > > > > > > this process change will help.
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > > Best Regards,
> > >> > > > > > >
> > >> > > > > > > Dmitriy Pavlov
> > >> > > > > > >
> > >> > > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <
> [hidden email]
> > >:
> > >> > > > > > >
> > >> > > > > > >> Vova,
> > >> > > > > > >>
> > >> > > > > > >> Contributors interested to make contributions and I
> propose
> > >> them
> > >> > > to
> > >> > > > > use
> > >> > > > > > >> *same* strategy as we (people inside the community) use.
> > >> > > > > > >> "-1" will not solve this issue, but my "tips" will.
> > >> > > > > > >>
> > >> > > > > > >> Dmitry,
> > >> > > > > > >>
> > >> > > > > > >> The main problem here is that nobody notified that
> someone
> > is
> > >> > > > waiting
> > >> > > > > > for
> > >> > > > > > >> the review.
> > >> > > > > > >> It's not a problem for me to provide tips or to make
> > review,
> > >> but
> > >> > > > it's
> > >> > > > > > >> problem to periodically check is there somebody waiting.
> > >> > > > > > >>
> > >> > > > > > >> Guys,
> > >> > > > > > >> Let's try this approach, and I'm pretty sure it will
> help.
> > >> > > > > > >>
> > >> > > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
> > >> > > > [hidden email]>
> > >> > > > > > >> wrote:
> > >> > > > > > >>
> > >> > > > > > >>> Hi Igniters, Anton,
> > >> > > > > > >>>
> > >> > > > > > >>> Let’s imagine that development process as a chain of
> > >> production
> > >> > > > > stages
> > >> > > > > > >>> 1) Developing patch by contributor
> > >> > > > > > >>> 2) Review changes by committer
> > >> > > > > > >>>
> > >> > > > > > >>> Reviews waiting too long time to be done at stage 2 may
> > >> > indicate
> > >> > > > that
> > >> > > > > > >> speed
> > >> > > > > > >>> (potential throughput) of step 2 is less than throughput
> > at
> > >> > step
> > >> > > 1.
> > >> > > > > > T2<T1
> > >> > > > > > >>>
> > >> > > > > > >>> In terms of this model (inspired by Goldratt’s Theory of
> > >> > > > Constraints
> > >> > > > > > >>> (TOC)), I have a question:
> > >> > > > > > >>> Will this responsibility movement (to find appropriate
> > >> reviewer
> > >> > > to
> > >> > > > > > >>> contributor) help us to increase overall throughput?
> > >> > > > > > >>>
> > >> > > > > > >>> If we agree constraint in terms of TOC is throughput
> T2, I
> > >> > > suggest
> > >> > > > > > >>> following steps
> > >> > > > > > >>> - Increase the throughput T2 of the committers
> > >> > > > > > >>> - Reduce the load on the committers by improve quality
> of
> > >> code
> > >> > > > after
> > >> > > > > > >> stage
> > >> > > > > > >>> 1 given to review (pre review by non-committer,
> automatic
> > >> > review,
> > >> > > > > code
> > >> > > > > > >>> inspections)
> > >> > > > > > >>>
> > >> > > > > > >>> Best Regards,
> > >> > > > > > >>> Dmitriy Pavlov
> > >> > > > > > >>>
> > >> > > > > > >>>
> > >> > > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <
> > [hidden email]
> > >> >:
> > >> > > > > > >>>
> > >> > > > > > >>>> Igniters,
> > >> > > > > > >>>>
> > >> > > > > > >>>> Currently, according to
> > >> > > > > > >>>>
> > >> > > > > > >>>> https://cwiki.apache.org/
> confluence/display/IGNITE/How+
> > >> > > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
> > >> > > > > > >>>> ,
> > >> > > > > > >>>> contributor can ask for review by moving ticket to
> PATCH
> > >> > > AVAILABLE
> > >> > > > > > >> state.
> > >> > > > > > >>>>
> > >> > > > > > >>>> And, as far as I can see, this cause broken tickets
> > issue.
> > >> > > > > > >>>> Contributor can wait somebody who'll review his changes
> > >> for a
> > >> > > > month
> > >> > > > > or
> > >> > > > > > >>> even
> > >> > > > > > >>>> more.
> > >> > > > > > >>>>
> > >> > > > > > >>>> I propose to change workflow and *make contributor
> > >> responsible
> > >> > > to
> > >> > > > > find
> > >> > > > > > >>>> reviewer*.
> > >> > > > > > >>>> It's pretty easy to find a person able to review
> changes
> > in
> > >> > most
> > >> > > > of
> > >> > > > > > >>> cases.
> > >> > > > > > >>>>
> > >> > > > > > >>>> 1) You can check git history of files you modified and
> > find
> > >> > > > persons
> > >> > > > > > >> with
> > >> > > > > > >>>> expertise in this code
> > >> > > > > > >>>> 2) In case you have problems with such search you can
> > >> always
> > >> > use
> > >> > > > > > >>>> maintainers list (
> > >> > > > > > >>>>
> > >> > > > > > >>>> https://cwiki.apache.org/
> confluence/display/IGNITE/How+
> > >> > > > > > >>> to+Contribute#HowtoContribute-
> ReviewProcessandMaintainers
> > >> > > > > > >>>> )
> > >> > > > > > >>>>
> > >> > > > > > >>>> Thoughts?
> > >> > > > > > >>>>
> > >> > > > > > >>>
> > >> > > > > > >>
> > >> > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Anton Vinogradov
Page updated.

On Wed, Jun 7, 2017 at 1:33 PM, Anton Vinogradov <[hidden email]>
wrote:

> Dmitriy,
>
> Got it,
> I'll add this as an optional "Tips to pass review quickly".
>
> On Tue, Jun 6, 2017 at 7:11 PM, Dmitriy Setrakyan <[hidden email]>
> wrote:
>
>> On Tue, Jun 6, 2017 at 7:40 AM, Anton Vinogradov <
>> [hidden email]>
>> wrote:
>>
>> > Igniters,
>> >
>> > Since we found that proposed approach can help,
>> > no one mind that I'll add text listed above to the wiki?
>> >
>>
>> I don't think that we have an agreement yet. Again, I still don't think it
>> is fair for a contributor to find a committer that has a relevant area of
>> expertise. A contributor should feel free to ask any committer for a
>> review, but it should not be mandatory. I would rather have an existing
>> contributor or committer help with finding a reviewer.
>>
>>
>> >
>> > On Tue, Jun 6, 2017 at 1:19 PM, Anton Vinogradov <
>> [hidden email]
>> > >
>> > wrote:
>> >
>> > > Dmitry,
>> > >
>> > > 1) See my initial email, it contains instruction how to find a
>> reviewer.
>> > > And it's pretty easy to do when you have something to review (you did
>> > some
>> > > code changes).
>> > >
>> > > I want to add following to our wiki:
>> > >
>> > > "
>> > > Ask commiter to review changes.
>> > > Check affected file's git history to find commiter most likely able to
>> > > review changes.
>> > > In case it's hard to determine who's able to review by git history use
>> > > maintainers list presented above.
>> > > Add "review request" comment to the ticket starting with a commiter
>> > > username.
>> > >
>> > > for example: "[~avinogradov], Please review my changes."
>> > >
>> > > Commiter will gain notification and review your changes and/or find
>> > > another commiter to do this.
>> > >
>> > > Important: Each comment should be started with [~username].
>> > > "
>> > >
>> > > 2) It will be a huge help to the community!
>> > >
>> > > On Tue, Jun 6, 2017 at 1:12 PM, Dmitry Pavlov <[hidden email]>
>> > > wrote:
>> > >
>> > >> Anton,
>> > >>
>> > >>
>> > >> Thank you for explanation. Personal ask instead of group broadcast
>> may
>> > >> really help. I understand the idea now.
>> > >>
>> > >>
>> > >> One argument against solution way 1) it may be not easy for
>> contributor,
>> > >> especially newcomer, to find a right person.
>> > >>
>> > >>
>> > >> What do you think about way 2? Personally, I'm ready to help with
>> > analysis
>> > >> and assignment of these 66 tasks from next week.
>> > >>
>> > >>
>> > >>
>> > >> вт, 6 июн. 2017 г. в 12:57, Anton Vinogradov <
>> [hidden email]
>> > >:
>> > >>
>> > >> > Dmitry Pavlov,
>> > >> >
>> > >> > There is *HUGE *difference between "Devlist, please review my
>> changes"
>> > >> > and "Dmitry Pavlov, please review my changes".
>> > >> >
>> > >> > In case you're busy right now, you'll, most likely, ignore appeal
>> to
>> > >> > devlist, but, I'm pretty sure, you'll check appeal to yourself.
>> > >> > Am I right?
>> > >> >
>> > >> > So, my idea is: appeal to devlist is a useless spam maker approach,
>> > but
>> > >> > appeal to person(s) works.
>> > >> >
>> > >> > On Tue, Jun 6, 2017 at 2:40 AM, Dmitriy Setrakyan <
>> > >> [hidden email]>
>> > >> > wrote:
>> > >> >
>> > >> > > Wow, we have 66 tickets waiting for review.... this is pretty
>> bad.
>> > >> > > Something must definitely change.
>> > >> > >
>> > >> > > From my side, having a contributor shop around for a reviewer is
>> not
>> > >> > really
>> > >> > > fair. However, I would support the idea of Apache Ignite
>> community
>> > >> > electing
>> > >> > > a person responsible to find reviewers for all contributions.
>> > >> > >
>> > >> > > D.
>> > >> > >
>> > >> > > On Mon, Jun 5, 2017 at 11:23 AM, Dmitry Pavlov <
>> > [hidden email]
>> > >> >
>> > >> > > wrote:
>> > >> > >
>> > >> > > > 1) There is waiting for review list here
>> > >> > > > https://cwiki.apache.org/confluence/display/IGNITE/
>> > >> > > > Issues+waiting+for+review
>> > >> > > >
>> > >> > > > 2) some of contributions are supplemented by dev-list messages
>> > >> (please
>> > >> > > > review my PR…)
>> > >> > > >
>> > >> > > >
>> > >> > > > And these two tools sometimes do not help. I suppose it is
>> because
>> > >> of
>> > >> > > > reviewers already have some things to do, but not because of
>> lack
>> > of
>> > >> > tool
>> > >> > > > support. Do you have other explanations?
>> > >> > > >
>> > >> > > >
>> > >> > > > But still, Igor’s suggestion to notify to dev list sounds
>> > >> reasonable to
>> > >> > > me.
>> > >> > > >
>> > >> > > >
>> > >> > > >
>> > >> > > > Anton, could it solve your requirement to know about issue
>> needed
>> > to
>> > >> > > > review?
>> > >> > > >
>> > >> > > > пн, 5 июн. 2017 г. в 21:06, Igor Sapego <[hidden email]
>> >:
>> > >> > > >
>> > >> > > > > By the way, there are emails being sent from Jira to devlist
>> > every
>> > >> > > > > time someone adds comment to ticket, or, for example, edits
>> its
>> > >> > > > > title which helps a lot to keep a track of ticket's state.
>> > >> > > > >
>> > >> > > > > But by some reason there is no such notification when ticket
>> > >> silently
>> > >> > > > > getting moved to "Patch available" state. I believe, that
>> would
>> > >> help
>> > >> > if
>> > >> > > > > there was a notification for that. Is it possible to
>> configure?
>> > >> > > > >
>> > >> > > > > Best Regards,
>> > >> > > > > Igor
>> > >> > > > >
>> > >> > > > > On Mon, Jun 5, 2017 at 9:00 PM, Denis Magda <
>> [hidden email]>
>> > >> > wrote:
>> > >> > > > >
>> > >> > > > > > In general, I tend to agree with Anton that something
>> needs to
>> > >> be
>> > >> > > > changed
>> > >> > > > > > in this direction.
>> > >> > > > > >
>> > >> > > > > > How many of you flip through dev list, JIRA or GitHub
>> > >> notifications
>> > >> > > in
>> > >> > > > an
>> > >> > > > > > attempt to find tickets that demand your attention? I bet
>> the
>> > >> > > > percentage
>> > >> > > > > is
>> > >> > > > > > pretty low.
>> > >> > > > > >
>> > >> > > > > > To solve this issue I see two options:
>> > >> > > > > > 1) Proposed by Anton.
>> > >> > > > > > 2) Having a guy in the community who’ll keep an eye on all
>> the
>> > >> > > incoming
>> > >> > > > > > pull-requests shuffling them between committer in the same
>> way
>> > >> > > proposed
>> > >> > > > > in
>> > >> > > > > > 1.
>> > >> > > > > >
>> > >> > > > > > Personally, I’m for 1.
>> > >> > > > > >
>> > >> > > > > > —
>> > >> > > > > > Denis
>> > >> > > > > >
>> > >> > > > > > > On Jun 5, 2017, at 10:28 AM, Dmitry Pavlov <
>> > >> > [hidden email]>
>> > >> > > > > > wrote:
>> > >> > > > > > >
>> > >> > > > > > > Hi Anton,
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > > > It is ok for me if it is advice and hint for faster
>> review,
>> > >> as it
>> > >> > > is
>> > >> > > > > now.
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > > > We can periodically remind about this opportunity at dev
>> > list
>> > >> or
>> > >> > in
>> > >> > > > the
>> > >> > > > > > > issue comments. We can remind that tasks in patch
>> available
>> > >> > status
>> > >> > > > may
>> > >> > > > > be
>> > >> > > > > > > reassigned by contributor.
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > > > Looking from prospective of overall throughput: it is not
>> > >> clear
>> > >> > for
>> > >> > > > me
>> > >> > > > > > how
>> > >> > > > > > > this process change will help.
>> > >> > > > > > >
>> > >> > > > > > >
>> > >> > > > > > > Best Regards,
>> > >> > > > > > >
>> > >> > > > > > > Dmitriy Pavlov
>> > >> > > > > > >
>> > >> > > > > > > пн, 5 июн. 2017 г. в 20:16, Anton Vinogradov <
>> [hidden email]
>> > >:
>> > >> > > > > > >
>> > >> > > > > > >> Vova,
>> > >> > > > > > >>
>> > >> > > > > > >> Contributors interested to make contributions and I
>> propose
>> > >> them
>> > >> > > to
>> > >> > > > > use
>> > >> > > > > > >> *same* strategy as we (people inside the community) use.
>> > >> > > > > > >> "-1" will not solve this issue, but my "tips" will.
>> > >> > > > > > >>
>> > >> > > > > > >> Dmitry,
>> > >> > > > > > >>
>> > >> > > > > > >> The main problem here is that nobody notified that
>> someone
>> > is
>> > >> > > > waiting
>> > >> > > > > > for
>> > >> > > > > > >> the review.
>> > >> > > > > > >> It's not a problem for me to provide tips or to make
>> > review,
>> > >> but
>> > >> > > > it's
>> > >> > > > > > >> problem to periodically check is there somebody waiting.
>> > >> > > > > > >>
>> > >> > > > > > >> Guys,
>> > >> > > > > > >> Let's try this approach, and I'm pretty sure it will
>> help.
>> > >> > > > > > >>
>> > >> > > > > > >> On Mon, Jun 5, 2017 at 7:58 PM, Dmitry Pavlov <
>> > >> > > > [hidden email]>
>> > >> > > > > > >> wrote:
>> > >> > > > > > >>
>> > >> > > > > > >>> Hi Igniters, Anton,
>> > >> > > > > > >>>
>> > >> > > > > > >>> Let’s imagine that development process as a chain of
>> > >> production
>> > >> > > > > stages
>> > >> > > > > > >>> 1) Developing patch by contributor
>> > >> > > > > > >>> 2) Review changes by committer
>> > >> > > > > > >>>
>> > >> > > > > > >>> Reviews waiting too long time to be done at stage 2 may
>> > >> > indicate
>> > >> > > > that
>> > >> > > > > > >> speed
>> > >> > > > > > >>> (potential throughput) of step 2 is less than
>> throughput
>> > at
>> > >> > step
>> > >> > > 1.
>> > >> > > > > > T2<T1
>> > >> > > > > > >>>
>> > >> > > > > > >>> In terms of this model (inspired by Goldratt’s Theory
>> of
>> > >> > > > Constraints
>> > >> > > > > > >>> (TOC)), I have a question:
>> > >> > > > > > >>> Will this responsibility movement (to find appropriate
>> > >> reviewer
>> > >> > > to
>> > >> > > > > > >>> contributor) help us to increase overall throughput?
>> > >> > > > > > >>>
>> > >> > > > > > >>> If we agree constraint in terms of TOC is throughput
>> T2, I
>> > >> > > suggest
>> > >> > > > > > >>> following steps
>> > >> > > > > > >>> - Increase the throughput T2 of the committers
>> > >> > > > > > >>> - Reduce the load on the committers by improve quality
>> of
>> > >> code
>> > >> > > > after
>> > >> > > > > > >> stage
>> > >> > > > > > >>> 1 given to review (pre review by non-committer,
>> automatic
>> > >> > review,
>> > >> > > > > code
>> > >> > > > > > >>> inspections)
>> > >> > > > > > >>>
>> > >> > > > > > >>> Best Regards,
>> > >> > > > > > >>> Dmitriy Pavlov
>> > >> > > > > > >>>
>> > >> > > > > > >>>
>> > >> > > > > > >>> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <
>> > [hidden email]
>> > >> >:
>> > >> > > > > > >>>
>> > >> > > > > > >>>> Igniters,
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> Currently, according to
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> https://cwiki.apache.org/confl
>> uence/display/IGNITE/How+
>> > >> > > > > > >>> to+Contribute#HowtoContribute-SubmittingforReview
>> > >> > > > > > >>>> ,
>> > >> > > > > > >>>> contributor can ask for review by moving ticket to
>> PATCH
>> > >> > > AVAILABLE
>> > >> > > > > > >> state.
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> And, as far as I can see, this cause broken tickets
>> > issue.
>> > >> > > > > > >>>> Contributor can wait somebody who'll review his
>> changes
>> > >> for a
>> > >> > > > month
>> > >> > > > > or
>> > >> > > > > > >>> even
>> > >> > > > > > >>>> more.
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> I propose to change workflow and *make contributor
>> > >> responsible
>> > >> > > to
>> > >> > > > > find
>> > >> > > > > > >>>> reviewer*.
>> > >> > > > > > >>>> It's pretty easy to find a person able to review
>> changes
>> > in
>> > >> > most
>> > >> > > > of
>> > >> > > > > > >>> cases.
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> 1) You can check git history of files you modified and
>> > find
>> > >> > > > persons
>> > >> > > > > > >> with
>> > >> > > > > > >>>> expertise in this code
>> > >> > > > > > >>>> 2) In case you have problems with such search you can
>> > >> always
>> > >> > use
>> > >> > > > > > >>>> maintainers list (
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> https://cwiki.apache.org/confl
>> uence/display/IGNITE/How+
>> > >> > > > > > >>> to+Contribute#HowtoContribute-
>> ReviewProcessandMaintainers
>> > >> > > > > > >>>> )
>> > >> > > > > > >>>>
>> > >> > > > > > >>>> Thoughts?
>> > >> > > > > > >>>>
>> > >> > > > > > >>>
>> > >> > > > > > >>
>> > >> > > > > >
>> > >> > > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> > >
>> > >
>> >
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: "Review workflow" changes to prevent "broken review" issues.

Антон Чураев
In reply to this post by Dmitriy Pavlov
Igniters, Dmitry, basically agree

It's no secret that Ignite is a very complex project, and contributors need
a very high experience to become a committer.
But this is a potential problem for scaling the community and project
development.

I think that one of the options for solving the growth problem could be to
decompose issues for new contributors. This will allow them to be included
in the project more quickly, and it will be more convenient for the
committers to code review.

There are cons, but what do you think?

2017-06-05 19:58 GMT+03:00 Dmitry Pavlov <[hidden email]>:

> Hi Igniters, Anton,
>
> Let’s imagine that development process as a chain of production stages
> 1) Developing patch by contributor
> 2) Review changes by committer
>
> Reviews waiting too long time to be done at stage 2 may indicate that speed
> (potential throughput) of step 2 is less than throughput at step 1. T2<T1
>
> In terms of this model (inspired by Goldratt’s Theory of Constraints
> (TOC)), I have a question:
> Will this responsibility movement (to find appropriate reviewer to
> contributor) help us to increase overall throughput?
>
> If we agree constraint in terms of TOC is throughput T2, I suggest
> following steps
> - Increase the throughput T2 of the committers
> - Reduce the load on the committers by improve quality of code after stage
> 1 given to review (pre review by non-committer, automatic review, code
> inspections)
>
> Best Regards,
> Dmitriy Pavlov
>
>
> пн, 5 июн. 2017 г. в 18:28, Anton Vinogradov <[hidden email]>:
>
> > Igniters,
> >
> > Currently, according to
> >
> > https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-SubmittingforReview
> > ,
> > contributor can ask for review by moving ticket to PATCH AVAILABLE state.
> >
> > And, as far as I can see, this cause broken tickets issue.
> > Contributor can wait somebody who'll review his changes for a month or
> even
> > more.
> >
> > I propose to change workflow and *make contributor responsible to find
> > reviewer*.
> > It's pretty easy to find a person able to review changes in most of
> cases.
> >
> > 1) You can check git history of files you modified and find persons with
> > expertise in this code
> > 2) In case you have problems with such search you can always use
> > maintainers list (
> >
> > https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-ReviewProcessandMaintainers
> > )
> >
> > Thoughts?
> >
>



--

Best Regards, Anton Churaev