Dmitry, can you please perform an excercise of looking that piece of code up (org.apache.ignite.internal.processors.cache.distributed.dht.atomic.GridDhtAtomicCache), telling us how an ideal message should look like in this case?
Thanks, -- Ilya Kasnacheev 2018-05-10 16:15 GMT+03:00 Dmitry Pavlov <dpavlov....@gmail.com>: > Hi Ilya, > > Error text `Unexpected exception during cache update` is brilliant example > how product should not behave. > > So requiring contributors to explain reasons of failure would be good first > step to come to situation that exception text is so clear that user will > know what to do. It will defenetely reduce number of messages to user list. > > Sincerely, > Dmitriy Pavlov > > > чт, 10 мая 2018 г. в 15:50, Ilya Kasnacheev <ilya.kasnach...@gmail.com>: > > > 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 > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > the > > > > > > > > > > >> > > > > > main > > > > > > > > > > >> > > > > > > > > > branch. > > > > > > > > > > >> > > > > > > > > > > > The > > > > > > > > > > >> > > > > > > > > > > > > > > > checklist would help us > to > > > > > detect > > > > > > a > > > > > > > > lot > > > > > > > > > of > > > > > > > > > > >> > common > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > problems > > > > > > > > > > >> > > > > > > > > such > > > > > > > > > > >> > > > > > > > > > a > > > > > > > > > > >> > > > > > > > > > > > > > broken > > > > > > > > > > >> > > > > > > > > > > > > > > > tests or bad UX earlier, > > and > > > > > would > > > > > > > > help > > > > > > > > > > >> > contributors > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > lead > > > > > > > > > > >> > > > > > > > > their > > > > > > > > > > >> > > > > > > > > > > pull > > > > > > > > > > >> > > > > > > > > > > > > > > > requests to merge. > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Hallmarks of a good > > > checklist: > > > > > > > > > > >> > > > > > > > > > > > > > > > - It must be followed be > > > > > everyone > > > > > > > > > without > > > > > > > > > > >> > exceptions > > > > > > > > > > >> > > > > > > > > > > > > > > > - It must be clear and > > > > disallow > > > > > > > > multiple > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > interpretations > > > > > > > > > > >> > > > > > > > > > > > > > > > - It must be > lightweight, > > > > > > otherwise > > > > > > > > > Ignite > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > development > > > > > > > > > > >> > > > > > > would > > > > > > > > > > >> > > > > > > > > > > become > > > > > > > > > > >> > > > > > > > > > > > a > > > > > > > > > > >> > > > > > > > > > > > > > > > nightmare > > > > > > > > > > >> > > > > > > > > > > > > > > > - It must be > non-blocking, > > > > i.e. > > > > > > > > > > >> inacessibility > > > > > > > > > > >> > of a > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > single > > > > > > > > > > >> > > > > > > > > > > > contributor > > > > > > > > > > >> > > > > > > > > > > > > > > > should not block ticket > > > > progress > > > > > > > > > forever. > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Please let me know if > you > > > > think > > > > > > the > > > > > > > > idea > > > > > > > > > > >> makes > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > sense. > > > > > > > > > > >> > > > > If > > > > > > > > > > >> > > > > > > we > > > > > > > > > > >> > > > > > > > > > agree > > > > > > > > > > >> > > > > > > > > > > on > > > > > > > > > > >> > > > > > > > > > > > > > it, > > > > > > > > > > >> > > > > > > > > > > > > > > > let's start defining > > action > > > > > items > > > > > > > for > > > > > > > > > the > > > > > > > > > > >> > checklist. > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > My > > > > > > > > > > >> > > > > > 2 > > > > > > > > > > >> > > > > > > > > cents: > > > > > > > > > > >> > > > > > > > > > > > > > > > 1) All unit tests pass > on > > TC > > > > > > without > > > > > > > > new > > > > > > > > > > >> > failures > > > > > > > > > > >> > > > > > > > > > > > > > > > 2) If ticket targets > > > specific > > > > > > > > component, > > > > > > > > > > it > > > > > > > > > > >> > should > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > be > > > > > > > > > > >> > > > > > > > reviewed > > > > > > > > > > >> > > > > > > > > > by > > > > > > > > > > >> > > > > > > > > > > > > > > > component's maintainer* > > > > > > > > > > >> > > > > > > > > > > > > > > > 3) If ticket changes > > public > > > > API > > > > > or > > > > > > > > > > existing > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > behavior, > > > > > > > > > > >> > > > > at > > > > > > > > > > >> > > > > > > > least > > > > > > > > > > >> > > > > > > > > > two > > > > > > > > > > >> > > > > > > > > > > > > > > > commiters should approve > > the > > > > > > changes > > > > > > > > ** > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Thoughts? > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Vladimir. > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > * TBD: Review component > > list > > > > and > > > > > > > > define > > > > > > > > > > >> > maintainers; > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > define > > > > > > > > > > >> > > > > > > > > what > > > > > > > > > > >> > > > > > > > > > > to > > > > > > > > > > >> > > > > > > > > > > > > > do if > > > > > > > > > > >> > > > > > > > > > > > > > > > maintainer is > unavailable > > > > > > > > > > >> > > > > > > > > > > > > > > > ** TBD: Define what is > > > "public > > > > > > API" > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > -- > > > > > > > > > > >> > > > > > > > > > Best regards, > > > > > > > > > > >> > > > > > > > > > Andrey Kuznetsov. > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > -- > > > > > > > > > > >> > > > Best regards, > > > > > > > > > > >> > > > Andrey Kuznetsov. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best regards, > > > > > > Andrey Kuznetsov. > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Best regards, > > > > Andrey Kuznetsov. > > > > > > > > > >
