Dmitry, In my experience refactoring without a reasons often produces more harm than benefits. Especially: 1) You may brake compatibility. E.g. moving anonymous inner class to the top level will cause renames of all other anonymous classes. 2) This may interfere severely with other people's work. If someone works on some issue in a separate branch, he will get a lot of severe conflicts when trying to merge his branch with master.
So I do not think we should do refactoring for refactoring. We should do this only for a strong reason. On Thu, Oct 12, 2017 at 5:12 PM, Дмитрий Рябов <somefire...@gmail.com> wrote: > IgniteKernal and some other classes have more places to refactor. At least > for better readable form or to remove inner class [1]. May be create some > tickets for refactoring such complex places? I mean to create main ticket > like [2] or [3], and when someone see something complex *and* the way to > refactor - he can create subtask/PR for refactoring. Thoughts? > > [1] - http://apache-ignite-developers.2346864.n4.nabble.com/Mini > mize-amount-of-inner-classes-predicates-tuples-etc-td17689.html > [2] - https://issues.apache.org/jira/browse/IGNITE-4575 > [3] - https://issues.apache.org/jira/browse/IGNITE-5156 > > > 2017-10-11 18:37 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > On Wed, Oct 11, 2017 at 7:44 AM, Иван Федотов <ivanan...@gmail.com> > wrote: > > > > > Hello, Igniters! > > > > > > I found, that in several places of IgniteKernal class code blocks are > > huge > > > and hard to understand and in other places methods have the same > context > > > and could be placed in their own class. For example methods: > > > “ackAsciiLogo”, “ackConfigUrl”, “ackDaemon”, “ackOsInfo”, > > > “ackLanguageRuntime”, “ackRemoteManagement”, “ackVmArguments”, > > > “ackClassPaths”, “ackSystemProperties”, “ackEnviromentVariables”, > > > “ackMemoryConfiguration”, “ackCacheConfiguration”, > “ackP2PConfiguration”, > > > “ackRebalanceConfiguration”, which are called in 800-813 lines and > > together > > > contain over than 250 lines, can be placed in separate class > > > “AckVariousInformation”. > > > > > > Do you think that it will be good to create task on such refactoring? > > > > > > > I think this is a matter of taste. One could argue that the code is more > > readable now because these methods belong to IgniteKernal and are not > > called from any other place. I would generally such changes. > > >
