ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vyacheslav Daradur <daradu...@gmail.com>
Subject Re: Code inspection
Date Sun, 09 Dec 2018 09:43:13 GMT
Hi, Maxim, Nikolay, I have the following questions regarding inspections:

Should 'gnite_inspections_teamcity.xml' been imported into IDEA, since
'ignite_inspections.xml' has been removed in actual master?

Also, I've faced mismatching: if I use
'@SuppressWarnings("ErrorNotRethrown")' in code, then this will be
marked on TC as "Redundant suppression". If I removed this suppression
in "main" code base (not in tests) then it's fine and IDE does not
mark the code by inspection. But, if I use
'GridTestUtils#assertThrows' in 'tests' code base, then IDE requires
to suppress the inspection, if I have done it then TC marks this as
"Redundant suppression".

What should I do in this case?

On Mon, Dec 3, 2018 at 10:26 PM Andrey Mashenkov
<andrey.mashenkov@gmail.com> wrote:
>
> 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



-- 
Best Regards, Vyacheslav D.

Mime
View raw message