ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Petr Ivanov <mr.wei...@gmail.com>
Subject Re: Code inspection
Date Mon, 11 Feb 2019 08:33:41 GMT
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 <nizhikov@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.weider@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 <nizhikov@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, Павлухин Иван <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
View raw message