ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Mashenkov <andrey.mashen...@gmail.com>
Subject Re: Code inspection
Date Mon, 03 Dec 2018 19:26:33 GMT
Hi,

Have someone tried to investigate the issue related to Inspection TC task
execution time variation (from 0.5 up to 1,5 hours)?
Can we enable GC logs for this task or may be even get CPU, Disk, Network
metrics?
Can someone check if there are unnecessary Idea plugins starts that can be
safely disabled?


On Tue, Nov 27, 2018 at 5:52 PM Dmitriy Pavlov <dpavlov@apache.org> wrote:

> I'm totally with you in this decision, let's move the file.
>
> вт, 27 нояб. 2018 г. в 16:24, Maxim Muzafarov <maxmuzaf@gmail.com>:
>
> > Igniters,
> >
> > I propose to make inspection configuration default on the project
> > level. I've created a new issue [1] for it. It can be easily done and
> > recommend by IntelliJ documentation [2].
> > Thoughts?
> >
> >
> > Vyacheslav,
> >
> > Can you share an example of your warnings?
> > Currently, we have different inspection configurations:
> > - ignite_inspections.xml - to import inspections as default and use it
> > daily.
> > - ignite_inspections_teamcity.xml - config to run it on TC. Only fixed
> > rules in the project code are enabled. Each of these rules are marked
> > with ERROR level.
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-10422
> > [2] https://www.jetbrains.com/help/idea/code-inspection.html
> > On Tue, 20 Nov 2018 at 13:58, Nikolay Izhikov <nizhikov@apache.org>
> wrote:
> > >
> > > Hello, Vyacheslav.
> > >
> > > Yes, we have.
> > >
> > > Maxim Muzafarov, can you fix it, please?
> > >
> > > вт, 20 нояб. 2018 г., 13:10 Vyacheslav Daradur daradurvs@gmail.com:
> > >
> > > > Guys, why we have 2 different inspection files in the repo?
> > > > idea\ignite_inspections.xml
> > > > idea\ignite_inspections_teamcity.xml
> > > >
> > > > AFAIK TeamCity is able to use the same inspection file with IDE.
> > > >
> > > > I've imported 'idea\ignite_inspections.xml' in the IDE, but now see
> > > > inspection warnings for my PR on TC because of different rules.
> > > >
> > > >
> > > > On Sun, Nov 11, 2018 at 6:06 PM Maxim Muzafarov <maxmuzaf@gmail.com>
> > > > wrote:
> > > > >
> > > > > Yakov, Dmitry,
> > > > >
> > > > > Which example of unsuccessful suite execution do we need?
> > > > > Does the current fail [1] in the master branch enough to configure
> > > > > notifications by TC.Bot?
> > > > >
> > > > > > Please consider adding more checks
> > > > > > - line endings. I think we should only have \n
> > > > > > - ensure blank line at the end of file
> > > > >
> > > > > It seems to me that `line endings` is easy to add, but for the
> `blank
> > > > > line at the end` we need as special regexp. Can we focus on
> built-in
> > > > > IntelliJ inspections at first and fix others special further?
> > > > >
> > > > > [1]
> > > >
> >
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv
> > > > > On Sun, 11 Nov 2018 at 17:55, Maxim Muzafarov <maxmuzaf@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > Igniters,
> > > > > >
> > > > > > Since the inspection rules are included in RunAll a few members
> of
> > the
> > > > > > community mentioned a wide distributed execution time on TC
> agents:
> > > > > >  - 1h:27m:38s publicagent17_9094
> > > > > >  - 38m:04s publicagent17_9094
> > > > > >  - 33m:29s publicagent17_9094
> > > > > >  - 17m:13s publicagent17_9094
> > > > > > It seems that we should configure the resources distribution
> > across TC
> > > > > > containers. Can anyone take a look at it?
> > > > > >
> > > > > >
> > > > > > I've also prepared the short list of rules to work on:
> > > > > > + Inconsistent line separators (6 matches)
> > > > > > + Problematic whitespace (4 matches)
> > > > > > + expression.equals("literal")' rather than
> > > > > > '"literal".equals(expression) (53 matches)
> > > > > > + Unnecessary 'null' check before 'instanceof' expression or
call
> > (42
> > > > matches)
> > > > > > + Redundant 'if' statement (69 matches)
> > > > > > + Redundant interface declaration (28 matches)
> > > > > > + Double negation (0 matches)
> > > > > > + Unnecessary code block (472 matches)
> > > > > > + Line is longer than allowed by code style (2614 matches) (Is
it
> > > > > > possible to implement?)
> > > > > >
> > > > > > WDYT?
> > > > > >
> > > > > > On Fri, 26 Oct 2018 at 23:43, Dmitriy Pavlov <
> > dpavlov.spb@gmail.com>
> > > > wrote:
> > > > > > >
> > > > > > > Hi Maxim,
> > > > > > >
> > > > > > >  thank you for your efforts to make this happen. Keep the
pace!
> > > > > > >
> > > > > > > Could you please provide an example of how Inspections
can
> fail,
> > so
> > > > I or
> > > > > > > another contributor could implement support of these failures
> > > > validation in
> > > > > > > the Tc Bot.
> > > > > > >
> > > > > > > Sincerely,
> > > > > > > Dmitriy Pavlov
> > > > > > >
> > > > > > > пт, 26 окт. 2018 г. в 18:27, Yakov Zhdanov <
> yzhdanov@apache.org
> > >:
> > > > > > >
> > > > > > > > Maxim,
> > > > > > > >
> > > > > > > > Thanks for response, let's do it the way you suggested.
> > > > > > > >
> > > > > > > > Please consider adding more checks
> > > > > > > > - line endings. I think we should only have \n
> > > > > > > > - ensure blank line in the end of file
> > > > > > > >
> > > > > > > > All these are code reviews issues I pointed out many
times
> when
> > > > reviewing
> > > > > > > > conributions. It would be cool if we have TC build
failing if
> > > > there is any.
> > > > > > > >
> > > > > > > > Thanks!
> > > > > > > >
> > > > > > > > --Yakov
> > > > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards, Vyacheslav D.
> > > >
> >
>


-- 
Best regards,
Andrey V. Mashenkov

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