Actually, I dont see anything wrong with failing *compilation* task. I think one should use project code style for everyday coding, not only for ready-to-merge PRs.
If we cant use code style for everyday coding, we should change the codestyle. What do you think? ср, 13 февр. 2019 г., 10:11 Petr Ivanov mr.wei...@gmail.com: > I guess that was about failing build configuration with Checkstype, not > compilation build itself. > > > On 12 Feb 2019, at 18:03, Павлухин Иван <vololo...@gmail.com> wrote: > > > > 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 > >
