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 &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
>> 

Reply via email to