Folks, Are you going to fail job compiling Ignite sources [1] if some inspection found a problem? Can we avoid it? It is quite common pattern to start some feature implementation with making a sketch and running tests against it. I found it convenient to skip some style requirements for such sketches (e.g. well formed javadocs).
[1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite пн, 11 февр. 2019 г. в 11:38, Nikolay Izhikov <nizhi...@apache.org>: > > Petr, we should have 1 configuration for project, may be 1 configuration > per programming language. > > пн, 11 февр. 2019 г., 11:33 Petr Ivanov mr.wei...@gmail.com: > > > I was asking about how many build configuration is intended? One for all > > and multiple per module? > > > > With IDEA inspections it was going to be build configuration per module. > > > > > > > > > > > On 11 Feb 2019, at 11:24, Nikolay Izhikov <nizhi...@apache.org> wrote: > > > > > > Hello, Petr. > > > > > > Are you saying that we have not single build task? And each module builds > > > when it required? If yes, then I propose to create a task like "Licence > > > check" which will be run for every patch. > > > > > > My point is that violation of codestyle should be treated as hard as > > > compile error. > > > > > > пн, 11 февр. 2019 г., 11:16 Petr Ivanov mr.wei...@gmail.com: > > > > > >> Is build configuration Inspections [Core] meant to transform into single > > >> all-modules check build configuration (without module subdivision)? > > >> > > >> > > >>> On 11 Feb 2019, at 11:02, Nikolay Izhikov <nizhi...@apache.org> wrote: > > >>> > > >>> 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, Павлухин Иван <vololo...@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 <oignate...@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 < > > >>>>> > > >>>>>> mr.weider@ > > >>>>> > > >>>>>> > 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 < > > >>>>> > > >>>>>> mr.weider@ > > >>>>> > > >>>>>> > wrote: > > >>>>>>>> > > >>>>>>>> Investigating problem, stand by. > > >>>>>>>> > > >>>>>>>> > > >>>>>>>>> On 18 Dec 2018, at 19:41, Dmitriy Pavlov < > > >>>>> > > >>>>>> dpavlov@ > > >>>>> > > >>>>>> > 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 > > >>>> > > >> > > >> > > > > -- Best regards, Ivan Pavlukhin
