ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Muzafarov <maxmu...@gmail.com>
Subject Re: Code inspection
Date Fri, 14 Dec 2018 08:13:21 GMT
Andrey,

Thanks! I've consulted with the IntelliJ IDEA source code and found
how this disabled plugins file should look like. I've created a new
issue [1] and prepared PR [2] with the set of disabled plugins (maybe
not complete set). I don't have access to change corresponding
`~Excluded [Inspections] Core Debug` test suite properties.
Can we test this PR?

[1] https://issues.apache.org/jira/browse/IGNITE-10682
[2] https://github.com/apache/ignite/pull/5666
On Thu, 13 Dec 2018 at 17:35, Andrey Mashenkov
<andrey.mashenkov@gmail.com> wrote:
>
> Maxim,
>
> Idea has a file in config directory ./config/disabled_plugins.txt , you can easily find
it at you local machine.
> Teamcity Inspections runner has an option "Disabled plugins" where disabled_plugins.txt
file content can be set.
>
> So, looks like we can disable useless plugins.
> But I'm not expert and can't suggest changes we can safely apply.
>
> On Thu, Dec 13, 2018 at 4:59 PM Maxim Muzafarov <maxmuzaf@gmail.com> wrote:
>>
>> Andrey,
>>
>> Thank you for solving this issue with GC pauses! I've checked the
>> given report. The inspections configuration is correct, but it seems
>> to me that we have enabled by default rules of included plugins (for
>> instance, KotlinInternalInJava in the report is enabled).
>>
>> Can you share more details about `disable plugin` option you found?
>>
>> I see that idea instance starts with the default -Didea.plugins.path
>> system property, can we change it so the plugins will be not loaded by
>> default?
>> On Thu, 13 Dec 2018 at 15:45, Andrey Mashenkov
>> <andrey.mashenkov@gmail.com> wrote:
>> >
>> > Maxim,
>> >
>> > It looks like we can't make logs more verbose due to possible bug, I've create
a ticket in Jetbrains Jira [1].
>> > We can just publish idea logs in artefacts as suggested in this manual [2].
>> >
>> > For now, Inspections logs looks like this one [3].
>> > Also, would you please to take a look at inspection report and check if we missed
smth and there are any unwanted inspection turned on.
>> >
>> > [1] https://youtrack.jetbrains.com/issue/TW-58422
>> > [2] https://confluence.jetbrains.com/display/TCD10/Reporting+Issues#ReportingIssues-IntelliJIDEAInspections
>> > [3] https://ci.ignite.apache.org/viewLog.html?buildId=2538111&buildTypeId=IgniteTests24Java8_ExcludedInspections2&tab=artifacts
>> >
>> > On Thu, Dec 13, 2018 at 3:19 PM Dmitriy Pavlov <dpavlov@apache.org> wrote:
>> >>
>> >> Maxim M, do you know if we can disable inspections by wildcard? E.g.
>> >> Android* ?
>> >>
>> >> чт, 13 дек. 2018 г. в 14:59, Andrey Mashenkov <andrey.mashenkov@gmail.com>:
>> >>
>> >> > Fixed memory issues with increasing heap size and forcing G1GC.
>> >> >
>> >> > Do we need all these plugins loaded for inspections?
>> >> > I've found a 'disable plugin' option in TC Inspections build configuration,
>> >> > but it is unclear how to disable plugin correctly.
>> >> > Can someone take over this?
>> >> >
>> >> > > 46 plugins initialized in 1031 ms
>> >> > > 2018-12-13 10:55:24,875 [ 1342] INFO - llij.ide.plugins.PluginManager
-
>> >> > > Loaded bundled plugins: Android Support (10.2.3), Ant Support
(1.0), CSS
>> >> > > Support (172.4574.11), Database Tools and SQL (172.4574.11), Eclipse
>> >> > > Integration (3.0), FreeMarker support (1.0), GWT Support (1.0),
Gradle
>> >> > > (172.4574.11), Groovy (9.0), Guice (8.0), HTML Tools (2.0), Hibernate
>> >> > > Support (1.0), I18n for Java (172.4574.11), IDEA CORE (172.4574.11),
>> >> > > IntelliLang (8.0), JBoss Seam Support (1.0), JUnit (1.0), Java
EE: Bean
>> >> > > Validation Support (1.1), Java EE: Contexts and Dependency Injection
>> >> > (1.1),
>> >> > > Java EE: EJB, JPA, Servlets (1.0), Java EE: Java Server Faces
(2.2.X.),
>> >> > > Java EE: Web Services (JAX-WS) (1.9), Java Server Pages (JSP)
Integration
>> >> > > (1.0), JavaScript Support (1.0), Kotlin (1.1.4-release-IJ2017.2-3),
Maven
>> >> > > Integration (172.4574.11), Persistence Frameworks Support (1.0),
Plugin
>> >> > > DevKit (1.0), Properties Support (172.4574.11), QuirksMode (172.4574.11),
>> >> > > Spring AOP/@AspectJ (1.0), Spring Batch (1.0), Spring Data (1.0),
Spring
>> >> > > Integration Patterns (1.0), Spring Security (1.0), Spring Support
(1.0),
>> >> > > Spring Web Flow (1.0), Spring Web Services (1.0), Struts 1.x (2.0),
>> >> > Struts
>> >> > > 2 (1.0), TestNG-J (8.0), UI Designer (172.4574.11), Velocity support
>> >> > (1.0),
>> >> > > W3C Validators (2.0), WebLogic Integration (1.0), XPathView +
XSLT
>> >> > Support
>> >> > > (4)
>> >> >
>> >> >
>> >> > Kotlin plugins fails to start, let's disable it.
>> >> >
>> >> > >
>> >> > > 2018-12-13 10:55:27,623 [   4090]   INFO -
>> >> > il.indexing.FileBasedIndexImpl - Rebuild requested for index
>> >> > org.jetbrains.kotlin.idea.versions.KotlinJvmMetadataVersionIndex
>> >> > > java.lang.Throwable
>> >> > >       at
>> >> > com.intellij.util.indexing.FileBasedIndex.requestRebuild(FileBasedIndex.java:68)
>> >> > >       at
>> >> > org.jetbrains.kotlin.idea.versions.KotlinUpdatePluginComponent.initComponent(KotlinUpdatePluginComponent.kt:54)
>> >> > >       at
>> >> > com.intellij.openapi.components.impl.ComponentManagerImpl$ComponentConfigComponentAdapter.getComponentInstance(ComponentManagerImpl.java:492)
>> >> > >       at
>> >> > com.intellij.openapi.components.impl.ComponentManagerImpl.createComponents(ComponentManagerImpl.java:118)
>> >> > >       at
>> >> > com.intellij.openapi.application.impl.ApplicationImpl.a(ApplicationImpl.java:462)
>> >> > >       at
>> >> > com.intellij.openapi.application.impl.ApplicationImpl.createComponents(ApplicationImpl.java:466)
>> >> > >       at
>> >> > com.intellij.openapi.components.impl.ComponentManagerImpl.init(ComponentManagerImpl.java:102)
>> >> > >       at
>> >> > com.intellij.openapi.application.impl.ApplicationImpl.load(ApplicationImpl.java:421)
>> >> > >       at
>> >> > com.intellij.openapi.application.impl.ApplicationImpl.load(ApplicationImpl.java:407)
>> >> > >       at com.intellij.idea.IdeaApplication.run(IdeaApplication.java:203)
>> >> >
>> >> >
>> >> >
>> >> > On Thu, Dec 13, 2018 at 1:45 PM Dmitriy Pavlov <dpavlov@apache.org>
wrote:
>> >> >
>> >> > > Sure, let's apply. I hope all TC agents may handle 4G heap.
>> >> > >
>> >> > > чт, 13 дек. 2018 г. в 12:54, Andrey Mashenkov <
>> >> > andrey.mashenkov@gmail.com
>> >> > > >:
>> >> > >
>> >> > > > Guys,
>> >> > > >
>> >> > > > I've just creates a copy of Inspections TC build task with
GC logs
>> >> > turned
>> >> > > > on to check if there is any issues
>> >> > > > and found Inspections task spent too much time in STW due
to long Full
>> >> > GC
>> >> > > > pauses.
>> >> > > >
>> >> > > > I've tried to increase Xmx up to 4Gb and use G1GC got 2+
times better
>> >> > > > execution time down to ~15 min (~17 for 2G heap).
>> >> > > > Increasing heap size only is not very helpful as it just
postpone Full
>> >> > GC
>> >> > > > issues, but changing GC to G1GC gives noticeable result.
>> >> > > >
>> >> > > > Let's apply this optimization.
>> >> > > > Thoughts?
>> >> > > >
>> >> > > >
>> >> > > > On Sun, Dec 9, 2018 at 12:43 PM Vyacheslav Daradur <
>> >> > daradurvs@gmail.com>
>> >> > > > wrote:
>> >> > > >
>> >> > > > > 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.
>> >> > > > >
>> >> > > >
>> >> > > >
>> >> > > > --
>> >> > > > Best regards,
>> >> > > > Andrey V. Mashenkov
>> >> > > >
>> >> > >
>> >> >
>> >> >
>> >> > --
>> >> > Best regards,
>> >> > Andrey V. Mashenkov
>> >> >
>> >
>> >
>> >
>> > --
>> > Best regards,
>> > Andrey V. Mashenkov
>
>
>
> --
> Best regards,
> Andrey V. Mashenkov

Mime
View raw message