Page updated. On Wed, Jun 7, 2017 at 1:33 PM, Anton Vinogradov <avinogra...@gridgain.com> 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 <dsetrak...@apache.org> > wrote: > >> On Tue, Jun 6, 2017 at 7:40 AM, Anton Vinogradov < >> avinogra...@gridgain.com> >> 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 < >> avinogra...@gridgain.com >> > > >> > 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 <dpavlov....@gmail.com> >> > > 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 < >> avinogra...@gridgain.com >> > >: >> > >> >> > >> > 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 < >> > >> dsetrak...@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....@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 <isap...@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 < >> dma...@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....@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 < >> a...@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....@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 < >> > a...@apache.org >> > >> >: >> > >> > > > > > >>> >> > >> > > > > > >>>> 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? >> > >> > > > > > >>>> >> > >> > > > > > >>> >> > >> > > > > > >> >> > >> > > > > > >> > >> > > > > > >> > >> > > > > >> > >> > > > >> > >> > > >> > >> > >> > >> >> > > >> > > >> > >> > >
