ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikolay Izhikov <nizhi...@apache.org>
Subject Re: Code inspection
Date Mon, 11 Feb 2019 08:02:08 GMT
Hello, Maxim.

+1 from me for migrating to checkstyle.

Oleg, there is plugin for IDEA with 2mln downloads -
https://plugins.jetbrains.com/plugin/1065-checkstyle-idea

I propose do the following:

1. Migrate current checks to checkstyle.
2. Apply checks to all Ignite modules. Currently, only core module are
checked.
I will review and commit this patch, or do it by my own.

3. Include code style checks to "Build Apache Ignite" suite. Ignite has to
fail to build if patch violates codestyle.

вс, 10 февр. 2019 г. в 07:54, Павлухин Иван <vololo100@gmail.com>:

> Hi,
>
> I also think that some warning from IDEA that some code style rule is
> violated is a must-have.
>
> вс, 10 февр. 2019 г. в 01:58, oignatenko <oignatenko@gridgain.com>:
> >
> > Hi Maxim,
> >
> > I believe that whatever style checks we establish at Teamcity, we better
> > take care of making it easy for developers to find and fix violations in
> > their typical dev environment (for Ignite this means, in IDEA). I think
> it
> > is important that developers can maintain required style with minimal
> effort
> > on their side.
> >
> > If above is doable then I am 200% for migrating our Teamcity inspections
> to
> > checkstyle / maven.
> >
> > This is because I am very disappointed observing how it stays broken for
> so
> > long. And worst of all, even when (if) it is fixed, I feel we will
> always be
> > at risk that it breaks again and that we will have to again wait for
> months
> > for it to be fixed.
> >
> > This is such a stark contrast with my experience regarding checkstyle
> based
> > inspections. These just work and you just never fear that it is going to
> > break for some obscure reason, this is so much better than what I observe
> > now.
> >
> > One suggestion in case if we pick checkstyle - I recommend keeping its
> > config file somewhere in the project under version control. I used to
> > maintain such a shared style config at one of past jobs and after some
> > experimenting it turned out most convenient to have it this way - so that
> > developers could easily assess and discuss style settings and keep track
> of
> > changes in these. (note how Kafka folks from your link [5] appear to be
> > doing it this way)
> >
> > regards, Oleg
> >
> >
> > Mmuzaf wrote
> > > Igniters,
> > >
> > > I've found that some of the community members have faced with
> > > `[Inspections] Core suite [1]` is not working well enough on TC. The
> > > suite has a `FAILED` status for more than 2 months due to some issues
> > > in TeamCity application [2]. Current suite behaviour confuses not only
> > > new contributors but also other community members. Moreover, this
> > > suite is no longer checks rules we previously configured. For
> > > instance, in the master branch, I've found 11 `Unused imports` which
> > > should have been caught earlier (e.g. for
> > > {{IgniteCachePutAllRestartTest} [3]).
> > >
> > > I think we should make the next step to enable an automatic code style
> > > checks. As an example, we can consider the Apache Kafka code style [5]
> > > way and configure for the Ignite project a maven-checkstyle-plugin
> > > with its own maven profile and run it simultaneously with other TC. We
> > > can also enable the previously configured inspection rules, so no
> > > coding style violations will be missed.
> > >
> > > I see some advantages of using a maven plugin:
> > > - an IDE agnostic way for code checks
> > > - can be used with different CI and build tools (Jenkins, TC)
> > > - executable from the command line
> > > - the entry single point to configure new rules
> > >
> > > I've created the ticket [4] and will prepare PR for it.
> > >
> > > WDYT?
> > >
> > > [1]
> > >
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv
> > > [2] https://youtrack.jetbrains.com/issue/TW-58504
> > > [3]
> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCachePutAllRestartTest.java#L29
> > > [4] https://issues.apache.org/jira/browse/IGNITE-11277
> > > [5] https://github.com/apache/kafka/tree/trunk/checkstyle
> > >
> > > On Fri, 21 Dec 2018 at 16:03, Petr Ivanov &lt;
> >
> > > mr.weider@
> >
> > > &gt; wrote:
> > >>
> > >> It seems there is bug in latest 2018.2 TeamCity
> > >> Bug is filed [1]
> > >>
> > >>
> > >> [1] https://youtrack.jetbrains.com/issue/TW-58504
> > >>
> > >> > On 19 Dec 2018, at 11:31, Petr Ivanov &lt;
> >
> > > mr.weider@
> >
> > > &gt; wrote:
> > >> >
> > >> > Investigating problem, stand by.
> > >> >
> > >> >
> > >> >> On 18 Dec 2018, at 19:41, Dmitriy Pavlov &lt;
> >
> > > dpavlov@
> >
> > > &gt; wrote:
> > >> >>
> > >> >> Both patches were applied. Maxim, thank you!
> > >> >>
> > >> >> What about 1. An `Unexpected error during build messages
> processing in
> > >> >> TeamCity`, what can we do as the next step to fix it?
> > >> >>
> > >> >> Sincerely,
> > >> >> Dmitriy Pavlov
> > >> >>[cut]
> > >> >
> > >>
> >
> >
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

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