Dmitry, I still do not see how new patches could be accepted with this requirement in place. Consider the following case: I created a patch and run it on TC, observed N failures, verified through TC history that none if them are new. Am I eligible to push the commit?
On Thu, May 24, 2018 at 3:11 PM, Dmitry Pavlov <dpavlov....@gmail.com> wrote: > Petr, good point. It is more intuitive, we should mark test we can ignore > by mute. > > So Vladimir, you or other Ignite veteran can mute test, if can say it is > not important. > > чт, 24 мая 2018 г. в 15:07, Petr Ivanov <mr.wei...@gmail.com>: > > > Why cannot we mute (and file corresponding tickets) all test failures > > (including flaky) to some date and start initiative Green TC? > > > > > > > > > On 24 May 2018, at 15:04, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > > > > Dmitry, > > > > > > We cannot add this requirements, because we do have failures on TC. > This > > > requirement implies that all development would stop until TC is green. > > > We never had old requirement work, neither we need to enforce it now. > > > > > > On Thu, May 24, 2018 at 2:59 PM, Dmitry Pavlov <dpavlov....@gmail.com> > > > wrote: > > > > > >> 3.c > > >> > > >> 1. All test suites *MUST* be run on TeamCity [3] before merge to > > master, > > >> there *MUST NOT* be any test failures > > >> > > >> > > >> 'New' word should be removed because we cant separate `new` and `non > > new` > > >> failures. > > >> > > >> Let's imagine example, we have 50 green runs in master. And PR Run-All > > >> contains this test failed. Is it new or not new? Actually we don't > know. > > >> > > >> Existing requirement is about all TC must be green, so let's keep it > as > > is. > > >> > > >> ср, 23 мая 2018 г. в 17:02, Vladimir Ozerov <voze...@gridgain.com>: > > >> > > >>> Igniters, > > >>> > > >>> I created review checklist on WIKI [1] and also fixed related pages > > (e.g. > > >>> "How To Contribute"). Please let me know if you have any comments > > before > > >> I > > >>> go with public announce. > > >>> > > >>> Vladimir. > > >>> > > >>> [1] > > https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist > > >>> > > >>> On Thu, May 10, 2018 at 5:10 PM, Vladimir Ozerov < > voze...@gridgain.com > > > > > >>> wrote: > > >>> > > >>>> Ilya, > > >>>> > > >>>> We define that exception messages *SHOULD* have clear explanation on > > >> what > > >>>> is wrong. *SHOULD* mean that the rule should be followed unless > there > > >> is > > >>> a > > >>>> reason not to follow. In your case you refer to some unexpected > > >> behavior. > > >>>> I.e. an exceptional situation developer is not aware of. In this > case > > >> for > > >>>> sure we cannot force contributor to explain what is wrong, because, > > >> well, > > >>>> we don't know. This is why we relaxed the rule from *MUST* to > > *SHOULD*. > > >>>> > > >>>> On Thu, May 10, 2018 at 3:50 PM, Ilya Kasnacheev < > > >>>> ilya.kasnach...@gmail.com> wrote: > > >>>> > > >>>>> I don't think I quite understand how exception explanations should > > >> work. > > >>>>> > > >>>>> Imagine we have the following exception: > > >>>>> > > >>>>> // At least RuntimeException can be thrown by the code above when > > >>>>> GridCacheContext is cleaned and there is > > >>>>> // an attempt to use cleaned resources. > > >>>>> U.error(log, "Unexpected exception during cache update", e); > > >>>>> > > >>>>> I mean, we genuinely don't know what happened here. > > >>>>> > > >>>>> Under new rules, what kind of "workaround" would that exception > > >> suggest? > > >>>>> "Try turning it off and then back on"? > > >>>>> What explanation how to resolve this exception can we offer? > "Please > > >>> write > > >>>>> to d...@apache.ignite.org or to Apache JIRA, and then wait for a > > >> release > > >>>>> with fix?" > > >>>>> > > >>>>> I'm really confused how we can implement 1.6 and 1.7 when dealing > > with > > >>>>> messy real-world code. > > >>>>> > > >>>>> Regards, > > >>>>> > > >>>>> > > >>>>> -- > > >>>>> Ilya Kasnacheev > > >>>>> > > >>>>> 2018-05-10 11:39 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > > >>>>> > > >>>>>> Andrey, Anton, Alex > > >>>>>> > > >>>>>> Agree, *SHOULD* is more appropriate here. > > >>>>>> > > >>>>>> Please see latest version below. Does anyone want to add or change > > >>>>>> something? Let's wait for several days for more feedback and then > > >>>>> publish > > >>>>>> and announce this list. Note that it would not be carved in stone > > >> and > > >>> we > > >>>>>> will be able to change it at any time if needed. > > >>>>>> > > >>>>>> 1) API > > >>>>>> 1.1) API compatibility *MUST* be maintained between minor > releases. > > >> Do > > >>>>> not > > >>>>>> remove existing methods or change their signatures, deprecate them > > >>>>> instead > > >>>>>> 1.2) Default behaviour "SHOULD NOT* be changed between minor > > >> releases, > > >>>>>> unless absolutely needed. If change is made, it *MUST* be > described > > >> in > > >>>>>> "Migration Guide" > > >>>>>> 1.3) New operation *MUST* be well-documented in code (javadoc, > > >>>>> dotnetdoc): > > >>>>>> documentation must contain method's purpose, description of > > >> parameters > > >>>>> and > > >>>>>> how their values affect the outcome, description of return value > and > > >>>>> it's > > >>>>>> default, behavior in negative cases, interaction with other > > >> operations > > >>>>> and > > >>>>>> components > > >>>>>> 1.4) API parity between Java and .NET platforms *SHOULD* be > > >> maintained > > >>>>> when > > >>>>>> operation makes sense on both platforms. If method cannot be > > >>>>> implemented in > > >>>>>> a platform immediately, new JIRA ticket *MUST* be created and > linked > > >>> to > > >>>>>> current ticket > > >>>>>> 1.5) API parity between thin clients (Java, .NET) *SHOULD* be > > >>> maintained > > >>>>>> when operation makes sense on several clients. If method cannot be > > >>>>>> implemented in a client immediately, new JIRA ticket *MUST* be > > >> created > > >>>>> and > > >>>>>> linked to current ticket > > >>>>>> 1.6) All exceptions thrown to a user **SHOULD** have explanation > how > > >>> to > > >>>>>> resolve, workaround or debug an error > > >>>>>> > > >>>>>> 2) Compatibility > > >>>>>> 2.1) Persistence backward compatibility *MUST* be maintained > between > > >>>>> minor > > >>>>>> releases. It should be possible to start newer version on data > files > > >>>>>> created by the previous version > > >>>>>> 2.2) Thin client forward and backward compatibility *SHOULD* be > > >>>>> maintained > > >>>>>> between two consecutive minor releases. If compatibility cannot be > > >>>>>> maintained it *MUST* be described in "Migration Guide" > > >>>>>> 2.3) JDBC and ODBC forward and backward compatibility *SHOULD* be > > >>>>>> maintained between two consecutive minor releases. If > compatibility > > >>>>> cannot > > >>>>>> be maintained it *MUST* be described in "Migration Guide" > > >>>>>> > > >>>>>> 3) Tests > > >>>>>> 3.1) New functionality *MUST* be covered with unit tests for both > > >>>>> positive > > >>>>>> and negative use cases > > >>>>>> 3.2) All test suites *MUST* be run before merge to master..There > > >>> *MUST* > > >>>>> be > > >>>>>> no new test failures > > >>>>>> > > >>>>>> 4) Code style *MUST* be followed as per Ignite's Coding Guidelines > > >>>>>> > > >>>>>> Vladimir. > > >>>>>> > > >>>>>> > > >>>>>> On Tue, May 8, 2018 at 1:05 PM, Andrey Kuznetsov < > stku...@gmail.com > > >>> > > >>>>>> wrote: > > >>>>>> > > >>>>>>> Anton, > > >>>>>>> > > >>>>>>> I agree, *MUST* for exception reasons and *SHOULD* for ways of > > >>>>> resolution > > >>>>>>> sound clearer. > > >>>>>>> > > >>>>>>> 2018-05-08 12:56 GMT+03:00 Anton Vinogradov <a...@apache.org>: > > >>>>>>> > > >>>>>>>> Andrey, > > >>>>>>>> > > >>>>>>>> How about > > >>>>>>>> 1.6) All exceptions thrown to a user *MUST* have explanation of > > >>>>>>> workaround > > >>>>>>>> and contain original error. > > >>>>>>>> All exceptions thrown to a user *SHOULD* have explanation how to > > >>>>>> resolve > > >>>>>>> if > > >>>>>>>> possible. > > >>>>>>>> ? > > >>>>>>>> > > >>>>>>>> вт, 8 мая 2018 г. в 12:26, Andrey Kuznetsov <stku...@gmail.com > > >>> : > > >>>>>>>> > > >>>>>>>>> Vladimir, checklist looks pleasant enough for me. > > >>>>>>>>> > > >>>>>>>>> I'd like to suggest one minor change. In 1.6 *MUST* seems to > > >> be > > >>>>> too > > >>>>>>>> strict, > > >>>>>>>>> *SHOULD* would be enough. It can be frustrating for API user > > >> if > > >>> I > > >>>>>>> explain > > >>>>>>>>> how to fix NPEs in a trivial way, for example. > > >>>>>>>>> > > >>>>>>>>> 2018-05-08 11:34 GMT+03:00 Anton Vinogradov <a...@apache.org>: > > >>>>>>>>> > > >>>>>>>>>> Alex, > > >>>>>>>>>> > > >>>>>>>>>> It is not sounds like that, obviously. > > >>>>>>>>>> > > >>>>>>>>>> Tests should cover all negative and positive cases. > > >>>>>>>>>> You should add enough tests to cover all cases. > > >>>>>>>>>> > > >>>>>>>>>> Sometimes one test can cover more than one case, so two > > >> tests > > >>>>> *CAN* > > >>>>>>>>>> partially check same things. > > >>>>>>>>>> In case some cases already covered you should not create > > >>>>>> duplicates. > > >>>>>>>>>> > > >>>>>>>>>> вт, 8 мая 2018 г. в 10:19, Александр Меньшиков < > > >>>>>> sharple...@gmail.com > > >>>>>>>> : > > >>>>>>>>>> > > >>>>>>>>>>> Vladimir, the 3.1 is a bit unclear for me. Which code > > >>>>> coverage is > > >>>>>>>>>>> acceptable? Now it sounds like two tests are enough (one > > >> for > > >>>>>>> positive > > >>>>>>>>> and > > >>>>>>>>>>> one for negative cases). > > >>>>>>>>>>> > > >>>>>>>>>>> 2018-05-07 23:09 GMT+03:00 Dmitriy Setrakyan < > > >>>>>>> dsetrak...@apache.org > > >>>>>>>>> : > > >>>>>>>>>>> > > >>>>>>>>>>>> Is this list on the Wiki? > > >>>>>>>>>>>> > > >>>>>>>>>>>> On Mon, May 7, 2018 at 7:26 AM, Vladimir Ozerov < > > >>>>>>>>> voze...@gridgain.com> > > >>>>>>>>>>>> wrote: > > >>>>>>>>>>>> > > >>>>>>>>>>>>> Igniters, > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> This is the checklist I have at the moment. Please let > > >>> me > > >>>>>> know > > >>>>>>> if > > >>>>>>>>> you > > >>>>>>>>>>>> have > > >>>>>>>>>>>>> any comments on existing items, or want to add or > > >> remove > > >>>>>>>> anything. > > >>>>>>>>> It > > >>>>>>>>>>>> looks > > >>>>>>>>>>>>> like we may have not only strict rules, but *nice to > > >>> have* > > >>>>>>> points > > >>>>>>>>>> here > > >>>>>>>>>>> as > > >>>>>>>>>>>>> well with help of *MUST*, *SHOULD* and *MAY* words as > > >>> per > > >>>>>>> RFC2119 > > >>>>>>>>>> [1]. > > >>>>>>>>>>> So > > >>>>>>>>>>>>> please feel free to suggest optional items as well. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> 1) API > > >>>>>>>>>>>>> 1.1) API compatibility *MUST* be maintained between > > >>> minor > > >>>>>>>> releases. > > >>>>>>>>>> Do > > >>>>>>>>>>>> not > > >>>>>>>>>>>>> remove existing methods or change their signatures, > > >>>>> deprecate > > >>>>>>>> them > > >>>>>>>>>>>> instead > > >>>>>>>>>>>>> 1.2) Default behaviour "SHOULD NOT* be changed between > > >>>>> minor > > >>>>>>>>>> releases, > > >>>>>>>>>>>>> unless absolutely needed. If change is made, it *MUST* > > >>> be > > >>>>>>>> described > > >>>>>>>>>> in > > >>>>>>>>>>>>> "Migration Guide" > > >>>>>>>>>>>>> 1.3) New operation *MUST* be well-documented in code > > >>>>>> (javadoc, > > >>>>>>>>>>>> dotnetdoc): > > >>>>>>>>>>>>> documentation must contain method's purpose, > > >> description > > >>>>> of > > >>>>>>>>>> parameters > > >>>>>>>>>>>> and > > >>>>>>>>>>>>> how their values affect the outcome, description of > > >>> return > > >>>>>>> value > > >>>>>>>>> and > > >>>>>>>>>>> it's > > >>>>>>>>>>>>> default, behavior in negative cases, interaction with > > >>>>> other > > >>>>>>>>>> operations > > >>>>>>>>>>>> and > > >>>>>>>>>>>>> components > > >>>>>>>>>>>>> 1.4) API parity between Java and .NET platforms > > >> *SHOULD* > > >>>>> be > > >>>>>>>>>> maintained > > >>>>>>>>>>>> when > > >>>>>>>>>>>>> operation makes sense on both platforms. If method > > >>> cannot > > >>>>> be > > >>>>>>>>>>> implemented > > >>>>>>>>>>>> in > > >>>>>>>>>>>>> a platform immediately, new JIRA ticket *MUST* be > > >>> created > > >>>>> and > > >>>>>>>>> linked > > >>>>>>>>>> to > > >>>>>>>>>>>>> current ticket > > >>>>>>>>>>>>> 1.5) API parity between thin clients (Java, .NET) > > >>>>> *SHOULD* be > > >>>>>>>>>>> maintained > > >>>>>>>>>>>>> when operation makes sense on several clients. If > > >> method > > >>>>>> cannot > > >>>>>>>> be > > >>>>>>>>>>>>> implemented in a client immediately, new JIRA ticket > > >>>>> *MUST* > > >>>>>> be > > >>>>>>>>>> created > > >>>>>>>>>>>> and > > >>>>>>>>>>>>> linked to current ticket > > >>>>>>>>>>>>> 1.6) All exceptions thrown to a user *MUST* have > > >>>>> explanation > > >>>>>>> how > > >>>>>>>> to > > >>>>>>>>>>>>> resolve, workaround or debug an error > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> 2) Compatibility > > >>>>>>>>>>>>> 2.1) Persistence backward compatibility *MUST* be > > >>>>> maintained > > >>>>>>>>> between > > >>>>>>>>>>>> minor > > >>>>>>>>>>>>> releases. It should be possible to start newer version > > >>> on > > >>>>>> data > > >>>>>>>>> files > > >>>>>>>>>>>>> created by the previous version > > >>>>>>>>>>>>> 2.2) Thin client forward and backward compatibility > > >>>>> *SHOULD* > > >>>>>> be > > >>>>>>>>>>>> maintained > > >>>>>>>>>>>>> between two consecutive minor releases. If > > >> compatibility > > >>>>>> cannot > > >>>>>>>> be > > >>>>>>>>>>>>> maintained it *MUST* be described in "Migration Guide" > > >>>>>>>>>>>>> 2.3) JDBC and ODBC forward and backward compatibility > > >>>>>> *SHOULD* > > >>>>>>> be > > >>>>>>>>>>>>> maintained between two consecutive minor releases. If > > >>>>>>>> compatibility > > >>>>>>>>>>>> cannot > > >>>>>>>>>>>>> be maintained it *MUST* be described in "Migration > > >>> Guide" > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> 3) Tests > > >>>>>>>>>>>>> 3.1) New functionality *MUST* be covered with unit > > >> tests > > >>>>> for > > >>>>>>> both > > >>>>>>>>>>>> positive > > >>>>>>>>>>>>> and negative use cases > > >>>>>>>>>>>>> 3.2) All test suites *MUST* be run before merge to > > >>>>>>> master..There > > >>>>>>>>>> *MUST* > > >>>>>>>>>>>> be > > >>>>>>>>>>>>> no new test failures > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> 4) Code style *MUST* be followed as per Ignite's > > >> Coding > > >>>>>>>> Guidelines > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Vladimir. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> [1] https://www.ietf.org/rfc/rfc2119.txt > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> On Fri, May 4, 2018 at 4:33 PM, Vladimir Ozerov < > > >>>>>>>>>> voze...@gridgain.com> > > >>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>> > > >>>>>>>>>>>>>> Hi Dmitry, > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> Yes, I'll do that in the nearest days. > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> On Wed, Apr 25, 2018 at 8:24 PM, Dmitry Pavlov < > > >>>>>>>>>>> dpavlov....@gmail.com> > > >>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> Igniters, the idea was related to small > > >> refactorings > > >>>>>>>> co-located > > >>>>>>>>>> with > > >>>>>>>>>>>>> main > > >>>>>>>>>>>>>>> change. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> Main change itself indicates that existing code did > > >>> not > > >>>>>> meet > > >>>>>>>> the > > >>>>>>>>>>>>> criteria > > >>>>>>>>>>>>>>> of practice. Approving of standalone refactorings > > >>>>> instead > > >>>>>>>>>>> contradicts > > >>>>>>>>>>>>> with > > >>>>>>>>>>>>>>> principle don't touch if it works. So I still like > > >>>>> idea of > > >>>>>>>>>>> co-located > > >>>>>>>>>>>>>>> changes improving code, javadocs, style, etc. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> But let's not argue about this point now, let's > > >>>>> summarize > > >>>>>>> the > > >>>>>>>>>>>> undisputed > > >>>>>>>>>>>>>>> points and add it to the wiki. Vladimir, would you > > >>>>> please > > >>>>>> do > > >>>>>>>> it? > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> ср, 25 апр. 2018 г. в 16:42, Nikolay Izhikov < > > >>>>>>>>> nizhi...@apache.org > > >>>>>>>>>>> : > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Igniters, > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> I agree with Vova. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Don't fix if it works! > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> If you 100% sure then it a useful addition to the > > >>>>>> product > > >>>>>>> - > > >>>>>>>>> just > > >>>>>>>>>>>> make > > >>>>>>>>>>>>> a > > >>>>>>>>>>>>>>>> separate ticket. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> В Ср, 25/04/2018 в 11:44 +0300, Vladimir Ozerov > > >>>>> пишет: > > >>>>>>>>>>>>>>>>> Guys, > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> The problem with in-place refactorings is that > > >>> you > > >>>>>>>> increase > > >>>>>>>>>>>> affected > > >>>>>>>>>>>>>>>> scope. > > >>>>>>>>>>>>>>>>> It is not uncommon to break compatibility or > > >>> public > > >>>>>>>>> contracts > > >>>>>>>>>>> with > > >>>>>>>>>>>>>>> even > > >>>>>>>>>>>>>>>>> minor things. E.g. recently we decided drop > > >>>>> org.jsr166 > > >>>>>>>>> package > > >>>>>>>>>>> in > > >>>>>>>>>>>>>>> favor > > >>>>>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>>> Java 8 classes. Innocent change. Result - > > >> broken > > >>>>>>> storage. > > >>>>>>>>>>> Another > > >>>>>>>>>>>>>>> problem > > >>>>>>>>>>>>>>>>> is conflicts. It is not uncommon to have > > >>> long-lived > > >>>>>>>> branches > > >>>>>>>>>>> which > > >>>>>>>>>>>>> we > > >>>>>>>>>>>>>>>> need > > >>>>>>>>>>>>>>>>> to merge with master over and over again. And a > > >>>>> lot of > > >>>>>>>>>>>> refactorings > > >>>>>>>>>>>>>>> cause > > >>>>>>>>>>>>>>>>> conflicts. It is much easier to resolve them if > > >>> you > > >>>>>> know > > >>>>>>>>> that > > >>>>>>>>>>>> logic > > >>>>>>>>>>>>>>> was > > >>>>>>>>>>>>>>>> not > > >>>>>>>>>>>>>>>>> affected as opposed to cases when you need to > > >>>>> resolve > > >>>>>>> both > > >>>>>>>>>>> renames > > >>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>> method extractions along with business-logic > > >>>>> changes. > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> I'd like to repeat - if you have a time for > > >>>>>> refactoring > > >>>>>>>> then > > >>>>>>>>>> you > > >>>>>>>>>>>>>>>> definitely > > >>>>>>>>>>>>>>>>> have a time to extract these changes to > > >> separate > > >>> PR > > >>>>>> and > > >>>>>>>>>> submit a > > >>>>>>>>>>>>>>> separate > > >>>>>>>>>>>>>>>>> ticket. I am quite understand what "low > > >> priority" > > >>>>> do > > >>>>>> you > > >>>>>>>>> mean > > >>>>>>>>>> if > > >>>>>>>>>>>> you > > >>>>>>>>>>>>>>> do > > >>>>>>>>>>>>>>>>> refactorings on your own. > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> On Tue, Apr 24, 2018 at 10:52 PM, Andrey > > >>> Kuznetsov > > >>>>> < > > >>>>>>>>>>>>> stku...@gmail.com > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> +1. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> Once again, I beg for "small refactoring > > >>>>> permission" > > >>>>>>> in > > >>>>>>>> a > > >>>>>>>>>>>>> checklist. > > >>>>>>>>>>>>>>>> As of > > >>>>>>>>>>>>>>>>>> today, separate tickets for small > > >> refactorings > > >>>>> has > > >>>>>>>> lowest > > >>>>>>>>>>>>> priority, > > >>>>>>>>>>>>>>>> since > > >>>>>>>>>>>>>>>>>> they neither fix any flaw nor add new > > >>>>> functionality. > > >>>>>>>> Also, > > >>>>>>>>>> the > > >>>>>>>>>>>>>>>> attempts to > > >>>>>>>>>>>>>>>>>> make issue-related code safer / cleaner / > > >> more > > >>>>>>> readable > > >>>>>>>> in > > >>>>>>>>>>>> "real" > > >>>>>>>>>>>>>>> pull > > >>>>>>>>>>>>>>>>>> requests are typically rejected, since they > > >>>>>> contradict > > >>>>>>>> our > > >>>>>>>>>>>> current > > >>>>>>>>>>>>>>>>>> guidelines. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> I understand this will require a bit more > > >>> effort > > >>>>>> from > > >>>>>>>>>>>>>>>> committer/maintainer, > > >>>>>>>>>>>>>>>>>> but otherwise we will get constantly > > >> degrading > > >>>>> code > > >>>>>>>>> quality. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> 2018-04-24 18:52 GMT+03:00 Eduard Shangareev > > >> < > > >>>>>>>>>>>>>>>> eduard.shangar...@gmail.com > > >>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>> Vladimir, > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> I am not talking about > > >> massive/sophisticated > > >>>>>>>>> refactoring. > > >>>>>>>>>>> But > > >>>>>>>>>>>> I > > >>>>>>>>>>>>>>>> believe > > >>>>>>>>>>>>>>>>>>> that ask to extract some methods should be > > >> OK > > >>>>> to > > >>>>>> do > > >>>>>>>>>> without > > >>>>>>>>>>> an > > >>>>>>>>>>>>>>> extra > > >>>>>>>>>>>>>>>>>>> ticket. > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> A checklist shouldn't be necessarily a set > > >> of > > >>>>>>> certain > > >>>>>>>>>> rules > > >>>>>>>>>>>> but > > >>>>>>>>>>>>>>> also > > >>>>>>>>>>>>>>>> it > > >>>>>>>>>>>>>>>>>>> could include suggestion and reminders. > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> On Tue, Apr 24, 2018 at 6:39 PM, Vladimir > > >>>>> Ozerov < > > >>>>>>>>>>>>>>>> voze...@gridgain.com> > > >>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> Ed, > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> Refactoring is a separate task. If you > > >>> would > > >>>>>> like > > >>>>>>> to > > >>>>>>>>>>> rework > > >>>>>>>>>>>>>>>> exchange > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> future > > >>>>>>>>>>>>>>>>>>>> - please do this in a ticket "Refactor > > >>>>> exchange > > >>>>>>>> task", > > >>>>>>>>>>>> nobody > > >>>>>>>>>>>>>>> would > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> against > > >>>>>>>>>>>>>>>>>>>> this. This is just a matter of creating > > >>>>> separate > > >>>>>>>>> ticket > > >>>>>>>>>>> and > > >>>>>>>>>>>>>>>> separate > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> PR. > > >>>>>>>>>>>>>>>>>>> If > > >>>>>>>>>>>>>>>>>>>> one have a time for refactoring, it > > >> should > > >>>>> not > > >>>>>> be > > >>>>>>> a > > >>>>>>>>>>> problem > > >>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>> him to > > >>>>>>>>>>>>>>>>>>>> spend several minutes on JIRA and GitHub. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> As far as documentation - what you > > >> describe > > >>>>> is > > >>>>>>>> normal > > >>>>>>>>>>> review > > >>>>>>>>>>>>>>>> process, > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> when > > >>>>>>>>>>>>>>>>>>>> reviewer might want to ask contributor to > > >>> fix > > >>>>>>>>> something. > > >>>>>>>>>>>>>>> Checklist > > >>>>>>>>>>>>>>>> is a > > >>>>>>>>>>>>>>>>>>>> different thing - this is a set of rules > > >>>>> which > > >>>>>>> must > > >>>>>>>> be > > >>>>>>>>>>>>> followed > > >>>>>>>>>>>>>>> by > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> anyone. > > >>>>>>>>>>>>>>>>>>>> I do not understand how you can define > > >>>>>>> documentation > > >>>>>>>>> in > > >>>>>>>>>>> this > > >>>>>>>>>>>>>>>> checklist. > > >>>>>>>>>>>>>>>>>>>> Same problem with logging - what is > > >>> "enough"? > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> On Tue, Apr 24, 2018 at 4:51 PM, Eduard > > >>>>>>> Shangareev < > > >>>>>>>>>>>>>>>>>>>> eduard.shangar...@gmail.com> wrote: > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Igniters, > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> I don't understand why you are so > > >> against > > >>>>>>>>> refactoring. > > >>>>>>>>>>>>>>>>>>>>> Code already smells like hell. Methods > > >>> 200+ > > >>>>>> line > > >>>>>>>> is > > >>>>>>>>>>>> normal. > > >>>>>>>>>>>>>>>> Exchange > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> future > > >>>>>>>>>>>>>>>>>>>>> is asking to be separated on several > > >> one. > > >>>>>>>>> Transaction > > >>>>>>>>>>> code > > >>>>>>>>>>>>>>> could > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> understand > > >>>>>>>>>>>>>>>>>>>>> few people. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> If we separate refactoring from > > >>>>> development it > > >>>>>>>> would > > >>>>>>>>>>> mean > > >>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>> no one > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> will > > >>>>>>>>>>>>>>>>>>>>> do it. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> 2) Documentation. > > >>>>>>>>>>>>>>>>>>>>> Everything which was asked by reviewers > > >>> to > > >>>>>>> clarify > > >>>>>>>>>> idea > > >>>>>>>>>>>>>>> should be > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> reflected > > >>>>>>>>>>>>>>>>>>>>> in the code. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> 3) Logging. > > >>>>>>>>>>>>>>>>>>>>> Logging should be enough to > > >> troubleshoot > > >>>>> the > > >>>>>>>> problem > > >>>>>>>>>> if > > >>>>>>>>>>>>>>> someone > > >>>>>>>>>>>>>>>> comes > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>> user-list with an issue in the code. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> On Fri, Apr 20, 2018 at 7:06 PM, Dmitry > > >>>>>> Pavlov < > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> dpavlov....@gmail.com> > > >>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Hi Igniters, > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> +1 to idea of checklist. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> +1 to refactoring and documenting > > >> code > > >>>>>> related > > >>>>>>>> to > > >>>>>>>>>>> ticket > > >>>>>>>>>>>>> in > > >>>>>>>>>>>>>>>> +/-20 > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> LOC > > >>>>>>>>>>>>>>>>>>>> at > > >>>>>>>>>>>>>>>>>>>>>> least. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> If we start to do it as part of our > > >>>>> regular > > >>>>>>>>>>>> contribution, > > >>>>>>>>>>>>>>> code > > >>>>>>>>>>>>>>>> will > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> be > > >>>>>>>>>>>>>>>>>>>>>> better, it would became common > > >> practice > > >>>>> and > > >>>>>>> part > > >>>>>>>>> of > > >>>>>>>>>>>> Apache > > >>>>>>>>>>>>>>>> Ignite > > >>>>>>>>>>>>>>>>>>>>>> development culure. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> If we will hope we will have free > > >> time > > >>> to > > >>>>>>> submit > > >>>>>>>>>>>> separate > > >>>>>>>>>>>>>>> patch > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> someday > > >>>>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>> have patience to complete > > >>>>> patch-submission > > >>>>>>>>> process, > > >>>>>>>>>>> code > > >>>>>>>>>>>>>>> will > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> remain > > >>>>>>>>>>>>>>>>>>>>>> undocumented and poor-readable. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Sincerely, > > >>>>>>>>>>>>>>>>>>>>>> Dmitriy Pavlov > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> пт, 20 апр. 2018 г. в 18:56, > > >> Александр > > >>>>>>>> Меньшиков < > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> sharple...@gmail.com > > >>>>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> 4) Metrics. > > >>>>>>>>>>>>>>>>>>>>>>> partially +1 > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> It makes sense to have some minimal > > >>>>> code > > >>>>>>>>> coverage > > >>>>>>>>>>> for > > >>>>>>>>>>>>> new > > >>>>>>>>>>>>>>>> code in > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> PR. > > >>>>>>>>>>>>>>>>>>>>>> IMHO. > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> Also, we can limit the cyclomatic > > >>>>>> complexity > > >>>>>>>> of > > >>>>>>>>>> the > > >>>>>>>>>>>> new > > >>>>>>>>>>>>>>> code > > >>>>>>>>>>>>>>>> in > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> PR > > >>>>>>>>>>>>>>>>>>>> too. > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> 6) Refactoring > > >>>>>>>>>>>>>>>>>>>>>>> -1 > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> I understand why people want to > > >>>>> refactor > > >>>>>> old > > >>>>>>>>> code. > > >>>>>>>>>>>>>>>>>>>>>>> But I think refactoring should be > > >>>>> always a > > >>>>>>>>>> separate > > >>>>>>>>>>>>> task. > > >>>>>>>>>>>>>>>>>>>>>>> And it's better to remove all > > >>>>> refactoring > > >>>>>>> from > > >>>>>>>>> PR, > > >>>>>>>>>>> if > > >>>>>>>>>>>>> it's > > >>>>>>>>>>>>>>>> not > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>> sense > > >>>>>>>>>>>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>>>>>>>>> the issue. > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> 2018-04-20 16:54 GMT+03:00 Andrey > > >>>>>> Kuznetsov > > >>>>>>> < > > >>>>>>>>>>>>>>>> stku...@gmail.com>: > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> What about adding the following > > >>> item > > >>>>> to > > >>>>>>> the > > >>>>>>>>>>>> checklist: > > >>>>>>>>>>>>>>>> when the > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> change > > >>>>>>>>>>>>>>>>>>>>>>> adds > > >>>>>>>>>>>>>>>>>>>>>>>> new functionality, then unit > > >> tests > > >>>>>> should > > >>>>>>>> also > > >>>>>>>>>> be > > >>>>>>>>>>>>>>>> provided, if > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> it's > > >>>>>>>>>>>>>>>>>>>>>>>> technically possible? > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> As for refactorings, in fact they > > >>> are > > >>>>>>>> strongly > > >>>>>>>>>>>>>>> discouraged > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> today > > >>>>>>>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>>>> some > > >>>>>>>>>>>>>>>>>>>>>>>> unclear reason. Let's permit to > > >>> make > > >>>>>>>>>> refactorings > > >>>>>>>>>>> in > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> checklist > > >>>>>>>>>>>>>>>>>>>>>> being > > >>>>>>>>>>>>>>>>>>>>>>>> discussed. (Of cource, > > >> refactoring > > >>>>>> should > > >>>>>>>>> relate > > >>>>>>>>>>> to > > >>>>>>>>>>>>>>> problem > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> being > > >>>>>>>>>>>>>>>>>>>>>>> solved.) > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> 2018-04-20 16:16 GMT+03:00 > > >> Vladimir > > >>>>>>> Ozerov < > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> voze...@gridgain.com > > >>>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> Hi Ed, > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> Unfortunately some of these > > >>> points > > >>>>> are > > >>>>>>> not > > >>>>>>>>>> good > > >>>>>>>>>>>>>>>> candidates > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>>>>> checklist because of these: > > >>>>>>>>>>>>>>>>>>>>>>>>> - It must be clear and disallow > > >>>>>>> *multiple > > >>>>>>>>>>>>>>>> interpretations* > > >>>>>>>>>>>>>>>>>>>>>>>>> - It must be *lightweight*, > > >>>>> otherwise > > >>>>>>>> Ignite > > >>>>>>>>>>>>>>> development > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> would > > >>>>>>>>>>>>>>>>>>>>>> become a > > >>>>>>>>>>>>>>>>>>>>>>>>> nightmare > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> We cannot have "nice to have" > > >>>>> points > > >>>>>>> here. > > >>>>>>>>>>>> Checklist > > >>>>>>>>>>>>>>>> should > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> answer > > >>>>>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>>>>> question "is ticket eligible to > > >>> be > > >>>>>>>> merged?" > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) Code style. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> +1 > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 2) Documentation > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> -1, it is impossible to define > > >>>>> what is > > >>>>>>>>>>>>>>>> "well-documented". A > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> piece > > >>>>>>>>>>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>>>>>>>>> code > > >>>>>>>>>>>>>>>>>>>>>>>>> could be obvious for one > > >>>>> contributor, > > >>>>>>> and > > >>>>>>>>>>>>> non-obvious > > >>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> another. > > >>>>>>>>>>>>>>>>>>>>> In > > >>>>>>>>>>>>>>>>>>>>>>> any > > >>>>>>>>>>>>>>>>>>>>>>>>> case this is not a blocker for > > >>>>> merge. > > >>>>>>>>> Instead, > > >>>>>>>>>>>>> during > > >>>>>>>>>>>>>>>> review > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> one > > >>>>>>>>>>>>>>>>>>>>> can > > >>>>>>>>>>>>>>>>>>>>>>> ask > > >>>>>>>>>>>>>>>>>>>>>>>>> implementer to add more docs, > > >> but > > >>>>> it > > >>>>>>>> cannot > > >>>>>>>>> be > > >>>>>>>>>>>>> forced. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) Logging > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> -1, same problem - what is > > >>> "enough > > >>>>>>>>> logging?". > > >>>>>>>>>>>> Enough > > >>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> whom? > > >>>>>>>>>>>>>>>>>>>> How > > >>>>>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>>>>>> understand whether it is enough > > >>> or > > >>>>>> not? > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 4) Metrics > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> -1, no clear boundaries, and > > >>>>> decision > > >>>>>> on > > >>>>>>>>>> whether > > >>>>>>>>>>>>>>> metrics > > >>>>>>>>>>>>>>>> are > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>> be > > >>>>>>>>>>>>>>>>>>>>>>> added > > >>>>>>>>>>>>>>>>>>>>>>>> or > > >>>>>>>>>>>>>>>>>>>>>>>>> not should be performed during > > >>>>> design > > >>>>>>>> phase. > > >>>>>>>>>> As > > >>>>>>>>>>>>>>> before, > > >>>>>>>>>>>>>>>> it is > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> perfectly > > >>>>>>>>>>>>>>>>>>>>>>>>> valid to ask contributor to add > > >>>>>> metrics > > >>>>>>>> with > > >>>>>>>>>>> clear > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> explanation > > >>>>>>>>>>>>>>>>>>>> why, > > >>>>>>>>>>>>>>>>>>>>>> but > > >>>>>>>>>>>>>>>>>>>>>>>>> this is not part of the > > >>> checklist. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 5) TC status > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> +1, already mentioned > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 6) Refactoring > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> Strong -1. OOP is a slippery > > >>> slope, > > >>>>>>> there > > >>>>>>>>> are > > >>>>>>>>>> no > > >>>>>>>>>>>>> good > > >>>>>>>>>>>>>>>> and bad > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> receipts > > >>>>>>>>>>>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>>>>>>> all cases, hence it cannot be > > >>> used > > >>>>> in > > >>>>>> a > > >>>>>>>>>>> checklist. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> We can borrow useful rules from > > >>>>> p.2, > > >>>>>> p.3 > > >>>>>>>> and > > >>>>>>>>>> p.4 > > >>>>>>>>>>>> if > > >>>>>>>>>>>>>>> you > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> provide > > >>>>>>>>>>>>>>>>>>>>> clear > > >>>>>>>>>>>>>>>>>>>>>>>>> definitions on how to measure > > >>> them. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> Vladimir. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Apr 20, 2018 at 3:50 > > >> PM, > > >>>>>> Eduard > > >>>>>>>>>>>> Shangareev < > > >>>>>>>>>>>>>>>>>>>>>>>>> eduard.shangar...@gmail.com> > > >>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> Also, I want to add some > > >>>>> technical > > >>>>>>>>>>> requirement. > > >>>>>>>>>>>>>>> Let's > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> discuss > > >>>>>>>>>>>>>>>>>>>>> them. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 1) Code style. > > >>>>>>>>>>>>>>>>>>>>>>>>>> The code needs to be > > >> formatted > > >>>>>>> according > > >>>>>>>>> to > > >>>>>>>>>>>> coding > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> guidelines > > >>>>>>>>>>>>>>>>>>>>>>>>>> < > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/ > > >>>>>>>>>> confluence/display/IGNITE/ > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> Coding+Guidelines > > >>>>>>>>>>>>>>>>>>>>>>>>> . > > >>>>>>>>>>>>>>>>>>>>>>>>>> The > > >>>>>>>>>>>>>>>>>>>>>>>>>> code must not contain TODOs > > >>>>> without > > >>>>>> a > > >>>>>>>>> ticket > > >>>>>>>>>>>>>>> reference. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> It is highly recommended to > > >>> make > > >>>>>> major > > >>>>>>>>>>>> formatting > > >>>>>>>>>>>>>>>> changes > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> in > > >>>>>>>>>>>>>>>>>>>>>> existing > > >>>>>>>>>>>>>>>>>>>>>>>>> code > > >>>>>>>>>>>>>>>>>>>>>>>>>> as a separate commit, to make > > >>>>> review > > >>>>>>>>> process > > >>>>>>>>>>>> more > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> practical. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 2) Documentation. > > >>>>>>>>>>>>>>>>>>>>>>>>>> Added code should be > > >>>>>> well-documented. > > >>>>>>>> Any > > >>>>>>>>>>>> methods > > >>>>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> raise > > >>>>>>>>>>>>>>>>>>>>>>> questions > > >>>>>>>>>>>>>>>>>>>>>>>>>> regarding their code flow, > > >>>>>> invariants, > > >>>>>>>>>>>>>>> synchronization, > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> etc., > > >>>>>>>>>>>>>>>>>>>>> must > > >>>>>>>>>>>>>>>>>>>>>> be > > >>>>>>>>>>>>>>>>>>>>>>>>>> documented with comprehensive > > >>>>>> javadoc. > > >>>>>>>> Any > > >>>>>>>>>>>>> reviewer > > >>>>>>>>>>>>>>> can > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> request > > >>>>>>>>>>>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>>>>>>>>> a > > >>>>>>>>>>>>>>>>>>>>>>>>>> particular added method be > > >>>>>> documented. > > >>>>>>>>> Also, > > >>>>>>>>>>> it > > >>>>>>>>>>>>> is a > > >>>>>>>>>>>>>>>> good > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> practice > > >>>>>>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>>>>>>> document old code in a 10-20 > > >>>>> lines > > >>>>>>>> region > > >>>>>>>>>>> around > > >>>>>>>>>>>>>>>> changed > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> code. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 3) Logging. > > >>>>>>>>>>>>>>>>>>>>>>>>>> Make sure that there are > > >> enough > > >>>>>>> logging > > >>>>>>>>>> added > > >>>>>>>>>>> in > > >>>>>>>>>>>>>>> every > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> category > > >>>>>>>>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>>>>>>>> possible diagnostic in field. > > >>>>> Check > > >>>>>>> that > > >>>>>>>>>>> logging > > >>>>>>>>>>>>>>>> messages > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> are > > >>>>>>>>>>>>>>>>>>>>>>> properly > > >>>>>>>>>>>>>>>>>>>>>>>>>> spelled. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 4) Metrics. > > >>>>>>>>>>>>>>>>>>>>>>>>>> Are there any metrics that > > >> need > > >>>>> to > > >>>>>> be > > >>>>>>>>>> exposed > > >>>>>>>>>>> to > > >>>>>>>>>>>>>>> user? > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 5) TC status. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> Recheck that there are no new > > >>>>>> failing > > >>>>>>>>> tests > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> 6) Refactoring. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> The code should be better > > >> than > > >>>>>> before: > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> - extract method from big > > >>> one; > > >>>>>>>>>>>>>>>>>>>>>>>>>> - do anything else to make > > >>>>> code > > >>>>>>>> clearer > > >>>>>>>>>>>> (don't > > >>>>>>>>>>>>>>>> forget > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> about > > >>>>>>>>>>>>>>>>>>>>> some > > >>>>>>>>>>>>>>>>>>>>>>>>>> OOP-practise, replace > > >>> if-else > > >>>>>> hell > > >>>>>>>> with > > >>>>>>>>>>>>>>> inheritance > > >>>>>>>>>>>>>>>>>>>>>>>>>> - split refactoring > > >>> (renaming, > > >>>>>> code > > >>>>>>>>>> format) > > >>>>>>>>>>>>> from > > >>>>>>>>>>>>>>>> actual > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> changes > > >>>>>>>>>>>>>>>>>>>>>> by > > >>>>>>>>>>>>>>>>>>>>>>>>>> separate commit > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Apr 20, 2018 at 3:23 > > >>> PM, > > >>>>>>> Eduard > > >>>>>>>>>>>>> Shangareev < > > >>>>>>>>>>>>>>>>>>>>>>>>>> eduard.shangar...@gmail.com> > > >>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, guys. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> I believe that we should > > >>> update > > >>>>>>>>>> maintainers > > >>>>>>>>>>>> list > > >>>>>>>>>>>>>>>> before > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> adding > > >>>>>>>>>>>>>>>>>>>>>> this > > >>>>>>>>>>>>>>>>>>>>>>>>>> review > > >>>>>>>>>>>>>>>>>>>>>>>>>>> requirement. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> There should not be the > > >>>>> situation > > >>>>>>> when > > >>>>>>>>>> there > > >>>>>>>>>>>> is > > >>>>>>>>>>>>>>> only > > >>>>>>>>>>>>>>>> one > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> contributor > > >>>>>>>>>>>>>>>>>>>>>>>>> who > > >>>>>>>>>>>>>>>>>>>>>>>>>>> is responsible for a > > >>> component. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> We already have issues with > > >>>>> review > > >>>>>>>> speed > > >>>>>>>>>> and > > >>>>>>>>>>>>>>> response > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> time. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Apr 20, 2018 at > > >> 2:17 > > >>>>> PM, > > >>>>>>> Anton > > >>>>>>>>>>>>> Vinogradov > > >>>>>>>>>>>>>>> < > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> a...@apache.org > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Vova, > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Everything you described > > >>>>> sound > > >>>>>>> good > > >>>>>>>> to > > >>>>>>>>>> me. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> I'd like to propose to > > >>> create > > >>>>>>>> special > > >>>>>>>>>> page > > >>>>>>>>>>>> at > > >>>>>>>>>>>>> AI > > >>>>>>>>>>>>>>>> Wiki > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>>>>> describe > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> checklist. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> In case we'll find > > >>> something > > >>>>>>> should > > >>>>>>>> be > > >>>>>>>>>>>>>>>> changed/improved > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> it > > >>>>>>>>>>>>>>>>>>>>> will > > >>>>>>>>>>>>>>>>>>>>>> be > > >>>>>>>>>>>>>>>>>>>>>>>>> easy > > >>>>>>>>>>>>>>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> update the page. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> 2018-04-20 0:53 GMT+03:00 > > >>>>>> Nikolay > > >>>>>>>>>> Izhikov > > >>>>>>>>>>> < > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> nizhi...@apache.org > > >>>>>>>>>>>>>>>>>>>>>>> : > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hello, Vladimir. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thank you for seting up > > >>>>> this > > >>>>>>>>>> discussion. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> As we discussed, I > > >> think > > >>> an > > >>>>>>>>> important > > >>>>>>>>>>> part > > >>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>> this > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> check > > >>>>>>>>>>>>>>>>>>>>> list > > >>>>>>>>>>>>>>>>>>>>>> is > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> compatibility rules. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> * What should be > > >> backward > > >>>>>>>>> compatible? > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> * How should we > > >> maintain > > >>>>> it? > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) If ticket changes > > >>>>> public > > >>>>>>> API > > >>>>>>>> or > > >>>>>>>>>>>>> existing > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> behavior, > > >>>>>>>>>>>>>>>>>>> at > > >>>>>>>>>>>>>>>>>>>>>> least > > >>>>>>>>>>>>>>>>>>>>>>>> two > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> commiters should > > >> approve > > >>>>> the > > >>>>>>>> changes > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> We can learn from other > > >>>>> open > > >>>>>>>> source > > >>>>>>>>>>>> project > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> experience. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Apache Kafka [1], for > > >>>>> example, > > >>>>>>>>>> requires > > >>>>>>>>>>>>>>> KIP(kafka > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> improvement > > >>>>>>>>>>>>>>>>>>>>>>>>>> proposal) > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> for *every* major > > >> change. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Major change definition > > >>>>>> includes > > >>>>>>>>>> public > > >>>>>>>>>>>> API. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1] > > >>>>> https://cwiki.apache.org/ > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> conf >
