Hi, Dmitry! To me, it's better to disable the following: Unnecessary 'this' qualifier -- this will, for example, warn on well-formed constructors. 'if' statement could be replaced with conditional expression -- let's decide on common sense basis whether it's appropriate, forceful refactorings could lead to non-readable code.
2018-03-30 18:57 GMT+03:00 Dmitry Pavlov <dpavlov....@gmail.com>: > Bumping up. Igniters, please reply and provide feedback on inspections > settings. > > I really prefer that we will merge inspections to codebase with clear > acknowledgment from active community members. > > чт, 29 мар. 2018 г. в 12:03, Alexey Goncharuk <alexey.goncha...@gmail.com > >: > > > From what I see, it should be rather easy to get a meaningful number of > > inspection failures to get something we can start working with. > > > > Namely, we have: > > Overly strong type cast (206) - mechanical work, easy to fix > > Assignment replaceable with operator assignment (23) - either mechanical > > work, or disable inspection > > 'expression.equals("literal")' rather than '"literal".equals(expression)' > > (49) - mechanical work > > 'size() == 0' replaceable with 'isEmpty()' (67) - mechanical work > > Missorted modifiers (121) - mechanical work > > Redundant field initialization (76) - mechanical work or disable > inspection > > Unnecessary 'this' qualifier (543) - mechanical work or disable > inspection > > 'if' statement could be replaced with conditional expression (244) - > > mechanical work or disable inspection > > Redundant throws declaration (100) - mechanical work or disable > inspection > > Redundant suppression (848) - mechanical work > > Missing @Override annotation (289) - mechanical work > > Property key/value delimiter doesn't match code style settings (2183) - > > disable inspection > > Unused Property (2180) - disable inspection > > > > For some of the inspections we have to agree whether we enforce a > > particular code style (for example, unnecessary 'this' qualifier). > > After this is done, the number of failed inspections will drop > dramatically > > and we can start tracking changes and pay more attention to other > > inspection categories. > > > > --AG > > > > 2018-03-28 21:19 GMT+03:00 Peter Ivanov <mr.wei...@gmail.com>: > > > > > Anton, Dmitry is right. > > > > > > We have to manually add condition when to consider build faulty based > on > > > how many failed inspection are there. > > > > > > For now I see this initiative as follows: > > > - find more or less correct set of inspections (there are lots of typos > > and > > > other irrelevant to code execution inspections) looking on the results > of > > > core module build, as it has ~85% of target code; > > > - add all modules to composite project and setup schedule at least > once a > > > week. > > > > > > > > > On Wed, 28 Mar 2018 at 19:09, Dmitry Pavlov <dpavlov....@gmail.com> > > wrote: > > > > > > > Inspection suites should be failed manually by some fail condition. > > > > > > > > This question will become actual in future. How to fail such suite on > > TC? > > > > > > > > ср, 28 мар. 2018 г. в 18:54, Anton Vinogradov <a...@apache.org>: > > > > > > > > > Peter, > > > > > > > > > > Why 44 errors are green? > > > > > > > > > > > > > > > > > > > https://ci.ignite.apache.org/viewLog.html?buildId=1145974& > > > tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_InspectionsAop > > > > > > > > > > 2018-03-28 16:27 GMT+03:00 Petr Ivanov <mr.wei...@gmail.com>: > > > > > > > > > > > After several problems, example run on Aleksey’s configuration is > > > > > > complete: > > https://ci.ignite.apache.org/viewLog.html?buildId=1164652 > > > < > > > > > > https://ci.ignite.apache.org/viewLog.html?buildId=1164652> > > > > > > > > > > > > > > > > > > > On 28 Mar 2018, at 10:28, Petr Ivanov <mr.wei...@gmail.com> > > wrote: > > > > > > > > > > > > > > Started > > https://ci.ignite.apache.org/viewLog.html?buildId=1164002 > > > < > > > > > > https://ci.ignite.apache.org/viewQueued.html?itemId=1163998> > with > > > > > > Aleksey’s inspections profile. > > > > > > > Core (long) and AOP (short) modules will be tested as example. > > > > > > > > > > > > > > > > > > > > > > > > > > > >> On 27 Mar 2018, at 19:38, Dmitry Pavlov < > dpavlov....@gmail.com > > > > > <mailto: > > > > > > dpavlov....@gmail.com>> wrote: > > > > > > >> > > > > > > >> Hi Petr, > > > > > > >> > > > > > > >> Could you please take inspections and run it on AI code base > in > > > > > > >> https://ci.ignite.apache.org/viewType.html?buildTypeId= > > > > > > > > > > > > IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault% > > > > > > 3E&tab=buildTypeStatusDiv <https://ci.ignite.apache.org/ > > > > > > > > viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_ > > > > > > IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv> > > > > > > >> ? > > > > > > >> > > > > > > >> Sincerely, > > > > > > >> Dmitriy Pavlov > > > > > > >> > > > > > > >> вт, 27 мар. 2018 г. в 19:27, Dmitry Pavlov < > > dpavlov....@gmail.com > > > >: > > > > > > >> > > > > > > >>> Alexey, thank you for bring this topic to top. > > > > > > >>> > > > > > > >>> What do you think about committing this inspections into > Ignite > > > > code > > > > > > base? > > > > > > >>> > > > > > > >>> What can be our next steps after demonstrating CI check is > > > possible > > > > > > >>> https://ci.ignite.apache.org/viewType.html?buildTypeId= > > > > > > > > > > > > IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault% > > > > > > 3E&tab=buildTypeStatusDiv > > > > > > >>> ? > > > > > > >>> > > > > > > >>> Sincerely, > > > > > > >>> Dmitriy Pavlov > > > > > > >>> > > > > > > >>> вт, 27 мар. 2018 г. в 15:28, Alexey Goncharuk < > > > > > > alexey.goncha...@gmail.com > > > > > > >>>> : > > > > > > >>> > > > > > > >>>> Bumping up. > > > > > > >>>> > > > > > > >>>> Attached is my local inspections profile exported from Idea. > > > Let's > > > > > run > > > > > > >>>> the first iteration and check if it differs significantly > from > > > > other > > > > > > >>>> community members. > > > > > > >>>> > > > > > > >>>> --AG > > > > > > >>>> > > > > > > >>>> 2018-03-19 16:39 GMT+03:00 Petr Ivanov <mr.wei...@gmail.com > >: > > > > > > >>>> > > > > > > >>>>> Filed https://issues.apache.org/jira/browse/IGNITE-7985 < > > > > > > >>>>> https://issues.apache.org/jira/browse/IGNITE-7985> [1]. > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>>>> On 18 Mar 2018, at 00:56, Dmitry Pavlov < > > > dpavlov....@gmail.com> > > > > > > wrote: > > > > > > >>>>>> > > > > > > >>>>>> Hello Petr, > > > > > > >>>>>> > > > > > > >>>>>> Many members of the community would appreciate such > > additional > > > > > code > > > > > > >>>>> control, and it's a pity that no one made this happen. > Agree? > > > > > > >>>>>> > > > > > > >>>>>> Could you please pick up this activity? > > > > > > >>>>>> > > > > > > >>>>>> It might be an idea to create 'IDEA Inspections' step to > be > > > run > > > > in > > > > > > >>>>> parallel with 'Build Apache Ignite'. WDYT? Would it work? > > > > > > >>>>>> > > > > > > >>>>>> Sincerely, > > > > > > >>>>>> Dmitriy Pavlov > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> https://confluence.jetbrains.com/display/TCD10/Inspections > < > > > > > > >>>>> https://confluence.jetbrains.com/display/TCD10/Inspections > > > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> пн, 12 мар. 2018 г. в 14:37, Dmitry Pavlov < > > > > dpavlov....@gmail.com > > > > > > >>>>> <mailto:dpavlov....@gmail.com>>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>> Hi Dmitriy, > > > > > > >>>>>> > > > > > > >>>>>> would you pick up this activity? > > > > > > >>>>>> > > > > > > >>>>>> Sincerely, > > > > > > >>>>>> Dmitriy Pavlov > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> вт, 6 мар. 2018 г. в 14:09, Dmitry Pavlov < > > > dpavlov....@gmail.com > > > > > > >>>>> <mailto:dpavlov....@gmail.com>>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>> What I can suggest now it is to take XML file with > existing > > as > > > > is > > > > > > from > > > > > > >>>>> previous topic (I remember someone in community already > > > prepared > > > > > > settings) > > > > > > >>>>> and set up TeamCity Run configuration as part of Run All > > Basic > > > > > Tests > > > > > > (per > > > > > > >>>>> commit basis). > > > > > > >>>>>> > > > > > > >>>>>> If we don’t have XML, I suggest to enable build-in Idea > > > > > inspections > > > > > > >>>>> 'as is' on TeamCity and iteratively improve it according to > > > found > > > > > > issues. > > > > > > >>>>>> > > > > > > >>>>>> Dmitriy G., would you prepare PR and proof-of-concept TC > run > > > > > > >>>>> configuration? > > > > > > >>>>>> > > > > > > >>>>>> As discussion became really active, I think that means > > > community > > > > > is > > > > > > >>>>> interested in static code checks. > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> вт, 6 мар. 2018 г. в 14:08, Dmitry Pavlov < > > > dpavlov....@gmail.com > > > > > > >>>>> <mailto:dpavlov....@gmail.com>>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>> I was thinking about some quick check, which will > > > automatically > > > > > > >>>>> require minimum runs. Now, any committer can push changes > to > > > the > > > > > > master, > > > > > > >>>>> which break not only the inspection and style, but even the > > > > > > compilation. If > > > > > > >>>>> this control would be automatic, it can allow us make > > codebase > > > > > > better quite > > > > > > >>>>> fast. But I am afraid it is not realistic. > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> вт, 6 мар. 2018 г. в 13:42, Petr Ivanov < > mr.wei...@gmail.com > > > > > > <mailto: > > > > > > >>>>> mr.wei...@gmail.com>>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>> Sonar is powerful, yes, but it’s power in thoroughness. > I.e. > > > it > > > > > does > > > > > > >>>>> its job well in cases of leisurely post-build analysis. > > > > > > >>>>>> > > > > > > >>>>>> I’d suggest we use it (if we will use it) in the following > > > > > > scenarios: > > > > > > >>>>>> — some basic checks Sonar profile for Blocker bugs (it is > > > fast) > > > > — > > > > > > >>>>> something that cannot be passed to master; > > > > > > >>>>>> — nightly or even weekly run with Full Sonar profile (600+ > > > > checks > > > > > > >>>>> from Firebug, Codestyle, Coverage, etc.) for regression and > > > > overall > > > > > > code > > > > > > >>>>> quality improvement goals. > > > > > > >>>>>> > > > > > > >>>>>> Did not quite get you about push-to-master prohibition. > Can > > > you > > > > > > >>>>> explain scenario in more details? > > > > > > >>>>>> > > > > > > >>>>>> > > > > > > >>>>>>> On 6 Mar 2018, at 13:27, Dmitry Pavlov < > > > dpavlov....@gmail.com > > > > > > >>>>> <mailto:dpavlov....@gmail.com>> wrote: > > > > > > >>>>>>> > > > > > > >>>>>>> Petr, I've heard Sonar is powerful tool. > > > > > > >>>>>>> > > > > > > >>>>>>> Would it help us to prohibit commits to master w/o test > > run / > > > > too > > > > > > >>>>> much > > > > > > >>>>>>> failed tests / too much inspection errors appeared? > > > > > > >>>>>>> > > > > > > >>>>> > > > > > > >>>>>> вт, 6 мар. 2018 г. в 13:22, Alexey Goncharuk < > > > > > > >>>>> alexey.goncha...@gmail.com <mailto: > > alexey.goncha...@gmail.com > > > >>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>>> > > > > > > >>>>>>>> Dmitriy, > > > > > > >>>>>>>> > > > > > > >>>>>>>> I like this idea a lot. For example, the inspection > > profile > > > > > should > > > > > > >>>>> have > > > > > > >>>>>>>> inspection 'Anonymous class can be converted to lambda' > > > > disabled > > > > > > >>>>> because > > > > > > >>>>>>>> quite a lot of such classes can be sent over the network > > > > > (although > > > > > > >>>>> even > > > > > > >>>>>>>> anonymous classes are discourage for such purposes). > > > > > > >>>>>>>> > > > > > > >>>>>>>> I believe we can start with sharing somehow one of the > > > > profiles > > > > > > and > > > > > > >>>>> then > > > > > > >>>>>>>> iteratively improving it until the community is > satisfied > > > with > > > > > the > > > > > > >>>>> result. > > > > > > >>>>>>>> > > > > > > >>>>>>>> Thoughts? > > > > > > >>>>>>>> > > > > > > >>>>> > > > > > > >>>>>>> 2018-03-06 12:06 GMT+03:00 Petr Ivanov < > > mr.wei...@gmail.com > > > > > > <mailto: > > > > > > >>>>> mr.wei...@gmail.com>>: > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>>>> > > > > > > >>>>>>>>> We can use Sonar as instrument for code analysis and > test > > > > > > coverage > > > > > > >>>>>>>>> inspections. > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>>>> On 6 Mar 2018, at 11:28, Dmitriy Govorukhin < > > > > > > >>>>>>>>> dmitriy.govoruk...@gmail.com <mailto: > dmitriy.govorukhin@ > > > > > > gmail.com>> > > > > > > >>>>> wrote: > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> Dmitriy, > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> As I understood, preview topic was of static code > > analysis > > > > in > > > > > > >>>>> general. > > > > > > >>>>>>>>>> In this topic, I want to discuss only idea inspection > > > rule. > > > > > > >>>>>>>>>> In future, of course, we can expаnd this rule to the > > > > TeamCity > > > > > > >>>>> build. > > > > > > >>>>>>>>>> > > > > > > >>>>> > > > > > > >>>>>>>>> On Tue, Mar 6, 2018 at 11:16 AM, Nikolay Izhikov < > > > > > > >>>>> nizhi...@apache.org <mailto:nizhi...@apache.org>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>>>>>> wrote: > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>>> Hello, Igniters. > > > > > > >>>>>>>>>>> > > > > > > >>>>>>>>>>> +1 to automatic code style tools. > > > > > > >>>>>>>>>>> > > > > > > >>>>>>>>>>> Let's make it already! > > > > > > >>>>>>>>>>> Do we have a ticket for it? > > > > > > >>>>>>>>>>> > > > > > > >>>>>>>>>>> Related discussion - > > > > > > >>>>> > > > > > > >>>>>>> http://apache-ignite-developers.2346864.n4.nabble < > > > > > > >>>>> http://apache-ignite-developers.2346864.n4.nabble/>. > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>>>>>>> com/Static-code-analysis-for-Java-td22195.html > > > > > > >>>>>>>>>>> > > > > > > >>>>>>>>>>> В Вт, 06/03/2018 в 08:15 +0000, Dmitry Pavlov пишет: > > > > > > >>>>>>>>>>>> Hi Dmitriy, > > > > > > >>>>>>>>>>>> > > > > > > >>>>>>>>>>>> I think we should resurrect thread about addition of > > > code > > > > > > >>>>>>>> inspections, > > > > > > >>>>>>>>>>> and > > > > > > >>>>>>>>>>>> later we can enable automatic control step to > > TeamCity. > > > > > > >>>>>>>>>>>> > > > > > > >>>>>>>>>>>> Could you help me to find it? > > > > > > >>>>>>>>>>>> > > > > > > >>>>>>>>>>>> вт, 6 мар. 2018 г. в 11:11, Dmitriy Govorukhin < > > > > > > >>>>> > > > > > > >>>>>>>>>> dmitriy.govoruk...@gmail.com <mailto: > > dmitriy.govorukhin@ > > > > > > gmail.com > > > > > > >>>>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>>>>>>>>>>> : > > > > > > >>>>>>>>>>>>> Hi folks, > > > > > > >>>>>>>>>>>>> > > > > > > >>>>>>>>>>>>> Do we have 'inspection' [1] scheme for ignite? > > > > > > >>>>>>>>>>>>> I see a lot of warnings in my code, and I guess it > is > > > > > because > > > > > > >>>>>>>> everyone > > > > > > >>>>>>>>>>> uses > > > > > > >>>>>>>>>>>>> different schemes. > > > > > > >>>>>>>>>>>>> > > > > > > >>>>>>>>>>>>> Let's start the discussion. > > > > > > >>>>>>>>>>>>> > > > > > > >>>>>>>>>>>>> [1] IDEA inspection > > > > > > >>>>> > > > > > > >>>>>>>>>>>> <https://www.jetbrains.com/ > > > help/idea/code-inspection.html > > > > < > > > > > > >>>>> https://www.jetbrains.com/help/idea/code-inspection.html>> > > > > > > >>>>>>>>>>>>> > > > > > > >>>>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> > > > > > > >>>> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Best regards, Andrey Kuznetsov.