ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov <avinogra...@gridgain.com>
Subject Re: "Review workflow" changes to prevent "broken review" issues.
Date Tue, 06 Jun 2017 09:57:45 GMT
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 <dsetrakyan@apache.org>
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 <dpavlov.spb@gmail.com>
> 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 <isapego@gridgain.com>:
> >
> > > 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 <dmagda@apache.org> 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 <dpavlov.spb@gmail.com>
> > > > 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 <av@apache.org>:
> > > > >
> > > > >> 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 <
> > dpavlov.spb@gmail.com>
> > > > >> 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 <av@apache.org>:
> > > > >>>
> > > > >>>> 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?
> > > > >>>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message