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