ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Goncharuk <alexey.goncha...@gmail.com>
Subject Re: Code inspection
Date Thu, 29 Mar 2018 09:02:54 GMT
>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.weider@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.spb@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 <av@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.weider@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.weider@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.spb@gmail.com
> > > <mailto:
> > > > dpavlov.spb@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.spb@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.goncharuk@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.weider@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.spb@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.spb@gmail.com
> > > > >>>>> <mailto:dpavlov.spb@gmail.com>>:
> > > > >>>>
> > > > >>>>
> > > > >>>>>> Hi Dmitriy,
> > > > >>>>>>
> > > > >>>>>> would you pick up this activity?
> > > > >>>>>>
> > > > >>>>>> Sincerely,
> > > > >>>>>> Dmitriy Pavlov
> > > > >>>>>>
> > > > >>>>>
> > > > >>>>> вт, 6 мар. 2018 г. в 14:09, Dmitry Pavlov
<
> dpavlov.spb@gmail.com
> > > > >>>>> <mailto:dpavlov.spb@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.spb@gmail.com
> > > > >>>>> <mailto:dpavlov.spb@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.weider@gmail.com
> > > > <mailto:
> > > > >>>>> mr.weider@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.spb@gmail.com
> > > > >>>>> <mailto:dpavlov.spb@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.goncharuk@gmail.com <mailto:alexey.goncharuk@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.weider@gmail.com
> > > > <mailto:
> > > > >>>>> mr.weider@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.govorukhin@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 <
> > > > >>>>> nizhikov@apache.org <mailto:nizhikov@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.govorukhin@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>>
> > > > >>>>>>>>>>>>>
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>
> > > > >>>>>
> > > > >>>>>
> > > > >>>>
> > > > >
> > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message