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/ >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> confluence/display/KAFKA/ >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>> Kafka+Improvement+Proposals >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> В Чт, 19/04/2018 в >> 23:00 >>>>>> +0300, >>>>>>>>>> Vladimir >>>>>>>>>>>>>>> Ozerov >>>>>>>>>>>>>>>> пишет: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Igniters, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It's glad to see our >>>>>> community >>>>>>>>>> becomes >>>>>>>>>>>>>>> larger >>>>>>>>>>>>>>>> every >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> day. >>>>>>>>>>>>>>>>>>>>> But >>>>>>>>>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>>>>>>>>> grows >>>>>>>>>>>>>>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> becomes more and more >>>>>>> difficult >>>>>>>> to >>>>>>>>>>>> manage >>>>>>>>>>>>>>>> review and >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> merge >>>>>>>>>>>>>>>>>>>>>>>>> processes >>>>>>>>>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> keep quality of our >>>>>> decisions >>>>>>> at >>>>>>>>> the >>>>>>>>>>>>> proper >>>>>>>>>>>>>>>> level. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> More >>>>>>>>>>>>>>>>>>>>>>>>>> contributors, >>>>>>>>>>>>>>>>>>>>>>>>>>>>> more >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> commits, more >>> components >>>>>>>>> interlinked >>>>>>>>>>>> with >>>>>>>>>>>>>>> each >>>>>>>>>>>>>>>> other >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>> subtle >>>>>>>>>>>>>>>>>>>>>>>>> ways. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I would like to >> propose >>>>> to >>>>>>>> setup a >>>>>>>>>>>> formal >>>>>>>>>>>>>>>> review >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> checklist. >>>>>>>>>>>>>>>>>>>>>>> This >>>>>>>>>>>>>>>>>>>>>>>>>> would >>>>>>>>>>>>>>>>>>>>>>>>>>>>> be a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> set of actions every >>>>>> reviewer >>>>>>>>> needs >>>>>>>>>> to >>>>>>>>>>>>> check >>>>>>>>>>>>>>>> before >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> approving >>>>>>>>>>>>>>>>>>>>>>>>> merge >>>>>>>>>>>>>>>>>>>>>>>>>>>> of a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> certain feature. >>> Passing >>>>> the >>>>>>>>>> checklist >>>>>>>>>>>>>>> would be >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> *necessary >>>>>>>>>>>>>>>>>>>>>> but >>>>>>>>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sufficient* phase >>> before >>>>>>> commit >>>>>>>>>> could >>>>>>>>>>> be >>>>>>>>>>>>>>> added >>>>>>>>>>>>>>>> to >>>>>>>>>>>> >>
