Dmitry, Could you please formulate this requirement?
On Wed, May 23, 2018 at 5:06 PM, Dmitry Pavlov <dpavlov....@gmail.com> wrote: > Hi Vladimir, > > I've replied in separate thread. > > I would like to keep requirement of Green TC instead of separation to > new/old test failures. > > Once you allow just one test failure, it would be more failures after some > time. > > Sincererly, > Dmitriy Pavlov > > ср, 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 > > >> > > > > > > > >
