Hi Igniters,
I understand it is not so Apache-way from my side to ask volunteers to do
some things (instead of contributing it by myself). But I still interested
- if we need code inspection or not,
- and who would like to spend odd hour and sett up some regular/manual
scans.
Sincerely,
Dmitriy Pavlov
пт, 30 мар. 2018 г. в 19:15, Andrey Kuznetsov <stku...@gmail.com>:
> 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.
>
