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 > >> > > > > > > >
