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 Tue, 23 Apr 2019 08:43:43 GMT
The suite is malformed.
If no ~/.m2/repository/org/apache/ignite artifact are installed in system, the build will fail [1]

It seems that we should use install instead of validate.


[1] https://ci.ignite.apache.org/viewLog.html?tab=buildLog&logTab=tree&filter=debug&expand=all&buildId=3677858&_focus=288 <https://ci.ignite.apache.org/viewLog.html?tab=buildLog&logTab=tree&filter=debug&expand=all&buildId=3677858&_focus=288>


> On 23 Apr 2019, at 00:25, Vyacheslav Daradur <daradurvs@gmail.com> wrote:
> 
> Maxim, I merged your changes to master.
> 
> Also, I've created a new build plan "Check Code Style" on TC [1] and
> included in RunAll build.
> The report of check-style attaches in artifacts once build is finished.
> 
> Please check that it works as expected once again and announce new
> requirements in a separate thread to avoid confusion of community.
> 
> cc Petr, Pavel (JFYI)
> 
> [1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_CheckCodeStyle&tab=buildTypeBranches
> 
> On Sun, Apr 21, 2019 at 10:18 PM Vyacheslav Daradur <daradurvs@gmail.com> wrote:
>> 
>> Maxim,
>> 
>> I left some comments in Jira, let's resolve them and I'll assist you
>> with merge and TC configuring.
>> 
>> On Fri, Apr 19, 2019 at 5:50 PM Maxim Muzafarov <maxmuzaf@gmail.com> wrote:
>>> 
>>> Vyacheslav,
>>> 
>>> Thank you for your interest in making Ignite coding style better.
>>> 
>>> The short answer is - there are no different checkstyle
>>> configurations. One for the JetBrains Inspections, and one the
>>> Checkstyle plugin. This is a completely different approach for
>>> checking the Ignites source code.
>>> 
>>> Currently, we have two different configurations for the JetBrains IDEA
>>> Inspection check:
>>> - ignite\.idea\inspectionProfiles\Project_Default.xml - this is
>>> default on the IDE level and used silently by every developer whos
>>> checkout Ignite project (it remains)
>>> - ignite\idea\ignite_inspections_teamcity.xml - this is the
>>> configuration of the inspection for the TC suite (it will be deleted)
>>> It's unobvious to maintain both of them. Previously we've planned to
>>> fix all the inspection rules one by one and add them one by one to the
>>> TC inspection configuration file (something like storing the
>>> intermediate result), but it didn't happen cause the inspection TC
>>> suite got broken after migration to 2018 version.
>>> 
>>> Now it seems to me, that it is better to use the best open source
>>> practices like checkstyle plugin (380K usages on github repositories)
>>> rather than proprietary software. So, we will:
>>> - keep IDE level inspection configuration (the Project_Default.xml config);
>>> - add a new checkstyle plugin configuration file (checkstyle.xml
>>> config) which will be used simultaneously for checking code style on
>>> build procedure and for the IDE-checkstyle plugin;
>>> 
>>> On Wed, 17 Apr 2019 at 21:23, Vyacheslav Daradur <daradurvs@gmail.com> wrote:
>>>> 
>>>> Maxim,
>>>> 
>>>> I looked through the PR and it looks good to me in general.
>>>> 
>>>> The only question how it's planned to maintain check styles in 2
>>>> different configurations, for IDEA and check style plugin?
>>>> 
>>>> On Mon, Mar 25, 2019 at 12:30 PM Maxim Muzafarov <maxmuzaf@gmail.com> wrote:
>>>>> 
>>>>> Igniters,
>>>>> 
>>>>> The issue [1] with enabled maven-checkstyle-plugin is ready for the review.
>>>>> All changes are prepared according to e-mail [2] the second option
>>>>> point (include the plugin in the maven build procedure by default).
>>>>> 
>>>>> JIRA: IGNITE-11277 [1]
>>>>> PR: [3]
>>>>> Upsource: [4]
>>>>> 
>>>>> How can take a look?
>>>>> 
>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-11277
>>>>> [2] http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-tp27709p41297.html
>>>>> [3] https://github.com/apache/ignite/pull/6119
>>>>> [4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1018
>>>>> 
>>>>> On Fri, 15 Mar 2019 at 19:19, Dmitriy Pavlov <dpavlov@apache.org> wrote:
>>>>>> 
>>>>>> Hi Igniters,
>>>>>> 
>>>>>> I see that a new TeamCity is released: Version: 2018.2.3.
>>>>>> 
>>>>>> Probably it could solve recently introduced problem related to:
>>>>>> Unexpected error during build messages processing in TeamCity;
>>>>>> 
>>>>>> Peter I., could you please check?
>>>>>> 
>>>>>> Sincerely,
>>>>>> Dmitriy Pavlov
>>>>>> 
>>>>>> пт, 15 мар. 2019 г. в 12:04, Павлухин Иван <vololo100@gmail.com>:
>>>>>> 
>>>>>>> I agree to gather some votes according to Maxim's proposal.
>>>>>>> 
>>>>>>> Personally, I will not put my vote here. Both options will work for me
>>>>>>> today.
>>>>>>> 
>>>>>>> But I would like to say a bit about agility. As I said both options
>>>>>>> sounds fine for me today. And I believe that we can switch from one to
>>>>>>> another easily in future. Let's do our best to be flexible.
>>>>>>> 
>>>>>>> пт, 15 мар. 2019 г. в 12:04, Павлухин Иван <vololo100@gmail.com>:
>>>>>>>> 
>>>>>>>> Maxim,
>>>>>>>> 
>>>>>>>>> As far as I understand your case, you want to fix all code styles
>>>>>>>> issues right before getting the final TC results. Right? ...
>>>>>>>> 
>>>>>>>> Actually, I mostly worry about accidental failures. For simple tasks
>>>>>>>> my workflow looks like:
>>>>>>>> 1. Create a branch.
>>>>>>>> 2. Write some code lines and tests.
>>>>>>>> 3. Run the most closely related tests from IDEA.
>>>>>>>> 4. Push changes to the branch.
>>>>>>>> 5. Launch TC.
>>>>>>>> 6. Take a cup of coffee ;-)
>>>>>>>> 7. Check TC results after a couple of hours.
>>>>>>>> 
>>>>>>>> And in such workflow I can accidentally leave styling error (IDEA does
>>>>>>>> not fail compilation). And I will receive not very valuable report
>>>>>>>> from TC. And will have to wait for another couple of hours.
>>>>>>>> 
>>>>>>>> Yes, usually I do not execute "mvn clean install" locally before
>>>>>>>> triggering TC. And I think that generally we should not do it because
>>>>>>>> TC does it.
>>>>>>>> 
>>>>>>>> If not everybody uses a bot visas it sounds bad for me. For patches
>>>>>>>> touching the code it should be mandatory. Also, as you might know
>>>>>>>> there are different kind of visas and for some trivial patches we can
>>>>>>>> request Checkstyle visa. Committers should check formalities.
>>>>>>>> 
>>>>>>>> пт, 15 мар. 2019 г. в 10:29, Nikolay Izhikov <nizhikov@apache.org>:
>>>>>>>>> 
>>>>>>>>> +1 to enable code style checks in compile time.
>>>>>>>>> 
>>>>>>>>> We can add option to disable maven codestyle profile with some
>>>>>>> environment variable.
>>>>>>>>> 
>>>>>>>>> Anyone who want violate common project rules in their local branch can
>>>>>>> set this variable and write some nasty code before push :)
>>>>>>>>> 
>>>>>>>>> пт, 15 марта 2019 г., 9:40 Maxim Muzafarov <maxmuzaf@gmail.com>:
>>>>>>>>>> 
>>>>>>>>>> Ivan,
>>>>>>>>>> 
>>>>>>>>>>> 1. I can write code and execute tests successfully even if there are
>>>>>>>>>> some style problems. I can imagine that a style error can arise
>>>>>>>>>> occasionally. And instead of getting test results after several hours
>>>>>>>>>> I will get a build failure without any tests run. I will simply lose
>>>>>>>>>> my time. But if the tests are allowed to proceed I will get a red flag
>>>>>>>>>> from code style check, fix those issues and rerun style check.
>>>>>>>>>> 
>>>>>>>>>> As far as I understand your case, you want to fix all code styles
>>>>>>>>>> issues right before getting the final TC results. Right? It's doable
>>>>>>>>>> you can disable checkstyle in your local branch and revet it back when
>>>>>>>>>> you've done with all your changes to get the final visa. But the
>>>>>>>>>> common case here is building the project locally and checking all
>>>>>>>>>> requirements for PR right before pushing it to the GitHub repo. I
>>>>>>>>>> always do so. The "Checklist before push" [1] have such
>>>>>>>>>> recommendations. Build the project before push will eliminate your use
>>>>>>>>>> case.
>>>>>>>>>> 
>>>>>>>>>> ---
>>>>>>>>>> 
>>>>>>>>>> Igniters,
>>>>>>>>>> 
>>>>>>>>>> To summarize the options we have with code checking behaviour:
>>>>>>>>>> 
>>>>>>>>>> 1)  (code style will be broken more often) Run checkstyle in the
>>>>>>>>>> separate TC suite and include it to the Bot visa.
>>>>>>>>>> - not all of us run TC for their branches especially for simple fixes
>>>>>>>>>> (it's the most common case when a new check style errors occur)
>>>>>>>>>> - not all of us use TC.Bot visa to verify their branches
>>>>>>>>>> - if this checkstyle suite starts to fail it will be ignored for some
>>>>>>>>>> time (not blocks the development process)
>>>>>>>>>> - a lot of suites for code checking (license, checkstyle, something
>>>>>>>>>> else in future)
>>>>>>>>>> 
>>>>>>>>>> + a bit comfortable way of TC tests execution for local\prototyped PRs
>>>>>>>>>> (it's a matter of taste)
>>>>>>>>>> + build the project and execute test suites a bit earlier (checkstyle
>>>>>>>>>> on the separate suite does not affect other suites)
>>>>>>>>>> 
>>>>>>>>>> 2) (code style will be broken less often) Run checkstyle on project
>>>>>>> build stage.
>>>>>>>>>> - increases a bit the build time procedure
>>>>>>>>>> - require additional operations to switch it off for prototyping
>>>>>>> branches
>>>>>>>>>> 
>>>>>>>>>> + do not require TC.Bot visa if someone of the community doesn't use
>>>>>>> it
>>>>>>>>>> + code style errors will be fixed immediately if the master branch
>>>>>>>>>> starts to fail
>>>>>>>>>> + the single place for code checks on maven code validation stage
>>>>>>>>>> (license check suite can be removed)
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Please, add another advantages\disadvantages that I've missed.
>>>>>>>>>> Let's vote and pick the most suitable option for the Apache Ignite
>>>>>>> needs.
>>>>>>>>>> 
>>>>>>>>>> ---
>>>>>>>>>> 
>>>>>>>>>> Personally, I'd like to choose the 2) option.
>>>>>>>>>> 
>>>>>>>>>> The JIRA [2] and PR [3] with the checkstyle enabled plugin is ready
>>>>>>>>>> for the review.
>>>>>>>>>> 
>>>>>>>>>> [1]
>>>>>>> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-Checklistbeforepush
>>>>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-11277
>>>>>>>>>> [3] https://github.com/apache/ignite/pull/6119
>>>>>>>>>> 
>>>>>>>>>> On Thu, 7 Mar 2019 at 11:19, Павлухин Иван <vololo100@gmail.com>
>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> Maxim,
>>>>>>>>>>> 
>>>>>>>>>>> From use cases described by you I use only 1 and 2. And actually I
>>>>>>>>>>> think that we can concentrate on 1 and forget about others for now.
>>>>>>>>>>> But please address my worries from previous letter:
>>>>>>>>>>> ====Quoted text====
>>>>>>>>>>> 1. I can write code and execute tests successfully even if there are
>>>>>>>>>>> some style problems. I can imagine that a style error can arise
>>>>>>>>>>> occasionally. And instead of getting test results after several
>>>>>>> hours
>>>>>>>>>>> I will get a build failure without any tests run. I will simply lose
>>>>>>>>>>> my time. But if the tests are allowed to proceed I will get a red
>>>>>>> flag
>>>>>>>>>>> from code style check, fix those issues and rerun style check.
>>>>>>>>>>> 2. Style check takes some time. With simple checks it is almost
>>>>>>>>>>> negligible. But it can grow if more checks are involved.
>>>>>>>>>>> ====End of quoted text====
>>>>>>>>>>> 
>>>>>>>>>>> Some clarifications. 1 is about running from IDEA first. 2 is
>>>>>>> related
>>>>>>>>>>> to opinions that we should involve much more checks, e.g. using
>>>>>>>>>>> abbreviations.
>>>>>>>>>>> 
>>>>>>>>>>> чт, 7 мар. 2019 г. в 10:36, Maxim Muzafarov <maxmuzaf@gmail.com>:
>>>>>>>>>>>> 
>>>>>>>>>>>> Ivan,
>>>>>>>>>>>> 
>>>>>>>>>>>> Let's take a look at all the options we have (ordered by the
>>>>>>> frequency of use):
>>>>>>>>>>>> 
>>>>>>>>>>>> 1. Check ready for merge branches (main case)
>>>>>>>>>>>> 2. Run tests on TC without checkstyle (prototyping branches)
>>>>>>>>>>>> 3. Local project build
>>>>>>>>>>>> 4. Quick build without any additional actions on TC
>>>>>>>>>>>> 
>>>>>>>>>>>> In the other projects (kafka, netty etc.) which I've checked the
>>>>>>> checkstyle plugin is used in the <build> section, so the user has no chance
>>>>>>> in common cases to disable it via command line (correct me if I'm wrong).
>>>>>>> In the PR [1] I've moved checkstyle configuration to the separate profile.
>>>>>>> I've set activation checkstyle profile if -DskipTests specified, so it will
>>>>>>> run with the basic build configuration. It can also be disabled locally if
>>>>>>> we really need it.
>>>>>>>>>>>> 
>>>>>>>>>>>> Back to our use cases:
>>>>>>>>>>>> 
>>>>>>>>>>>> 1. For checking the ready to merge branches we will fail the
>>>>>>> ~Build Apache Ignite~ suite, so no configured checkstyle rules will be
>>>>>>> violated. If we will use the TC.Bot approach someone will merge the branch
>>>>>>> without running TC.Bot on it, but no one will merge the branch with compile
>>>>>>> errors.
>>>>>>>>>>>> 
>>>>>>>>>>>> 2. For the prototyping branches, you can turn off checkstyle in
>>>>>>> your local PR by removing activation configuration. It's ok as these type
>>>>>>> of branches will never be merged to the master.
>>>>>>>>>>>> 
>>>>>>>>>>>> 3. From my point, local builds should be always run with the
>>>>>>> checkstyle enabled profile. The common build action as `mvn clean install
>>>>>>> -DskipTests` will activate the profile.
>>>>>>>>>>>> 
>>>>>>>>>>>> 4. The checkstyle profile can be disabled explicitly on TC by
>>>>>>> specifying -P !checkstyle option. A don't see any use cases of it, but it's
>>>>>>> completely doable.
>>>>>>>>>>>> 
>>>>>>>>>>>> Please, correct me if I've missed something.
>>>>>>>>>>>> 
>>>>>>>>>>>> I propose to merge PR [1] as it is, with the configured set of
>>>>>>> rules.
>>>>>>>>>>>> 
>>>>>>>>>>>> [1] https://github.com/apache/ignite/pull/6119
>>>>>>>>>>>> 
>>>>>>>>>>>> On Tue, 5 Mar 2019 at 19:02 Павлухин Иван <vololo100@gmail.com>
>>>>>>> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Maxim,
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I like an idea of being IDE agnostic. I am ok with currently
>>>>>>> enabled
>>>>>>>>>>>>> checks, they are a must-have in my opinion (even for prototypes).
>>>>>>>>>>>>> 
>>>>>>>>>>>>> But I am still do not like an idea of preventing tests execution
>>>>>>> if
>>>>>>>>>>>>> style check finds a problem. I checked out PR, installed a
>>>>>>> plugin and
>>>>>>>>>>>>> tried it out. Here are my concerns:
>>>>>>>>>>>>> 1. I can write code and execute tests successfully even if there
>>>>>>> are
>>>>>>>>>>>>> some style problems. I can imagine that a style error can arise
>>>>>>>>>>>>> occasionally. And instead of getting test results after several
>>>>>>> hours
>>>>>>>>>>>>> I will get a build failure without any tests run. I will simply
>>>>>>> lose
>>>>>>>>>>>>> my time. But if the tests are allowed to proceed I will get a
>>>>>>> red flag
>>>>>>>>>>>>> from code style check, fix those issues and rerun style check.
>>>>>>>>>>>>> 2. Style check takes some time. With simple checks it is almost
>>>>>>>>>>>>> negligible. But it can grow if more checks are involved.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> On the bright side I found nice integration of Checkstyle plugin
>>>>>>> with
>>>>>>>>>>>>> IDEA commit dialog. There is a checkbox "Scan with Checkstyle"
>>>>>>> which I
>>>>>>>>>>>>> think is quite useful.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> пн, 4 мар. 2019 г. в 15:00, Maxim Muzafarov <maxmuzaf@gmail.com
>>>>>>>> :
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Ivan,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I like that Jetbrains inspections are integrated with IDE and
>>>>>>> TC out
>>>>>>>>>>>>>> of the box, but currently, they are working not well enough on
>>>>>>> TC.
>>>>>>>>>>>>>> Actually, they are not checking our source code at all.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Let's try a bit another approach and try to be IDE-agnostic
>>>>>>> with code
>>>>>>>>>>>>>> style checking. I've checked popular java projects: hadoop,
>>>>>>> kafka,
>>>>>>>>>>>>>> spark, hive, netty. All of them are using
>>>>>>> maven-checkstyle-plugin in
>>>>>>>>>>>>>> their <build> section by default, so why don't we? It sounds
>>>>>>>>>>>>>> reasonable for me at least to try so.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Can you take a look at my changes below?
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Igniters,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> PR [2] has been prepared. All the details I've mentioned in my
>>>>>>> comment
>>>>>>>>>>>>>> in JIRA [4].
>>>>>>>>>>>>>> Can anyone take a look at my changes?
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> JIRA: [1]
>>>>>>>>>>>>>> PR: [2]
>>>>>>>>>>>>>> Upsource: [3]
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Questions to discuss:
>>>>>>>>>>>>>> 1) There is no analogue for inspections RedundantSuppression
>>>>>>> and
>>>>>>>>>>>>>> SizeReplaceableByIsEmpty (all code style checks [5]). Propose
>>>>>>> to merge
>>>>>>>>>>>>>> without them.
>>>>>>>>>>>>>> 2) Checkstyle plugin has it's own maven profile and enabled by
>>>>>>>>>>>>>> default. It can be turned off for prototype branches.
>>>>>>>>>>>>>> 3) I've removed the inspections configuration for the TC suite
>>>>>>> and
>>>>>>>>>>>>>> propose to disable it as not working.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-11277
>>>>>>>>>>>>>> [2] https://github.com/apache/ignite/pull/6119
>>>>>>>>>>>>>> [3]
>>>>>>> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1018
>>>>>>>>>>>>>> [4]
>>>>>>> https://issues.apache.org/jira/browse/IGNITE-11277?focusedCommentId=16771200&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16771200
>>>>>>>>>>>>>> [5] http://checkstyle.sourceforge.net/checks.html
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Thu, 14 Feb 2019 at 16:21, Павлухин Иван <
>>>>>>> vololo100@gmail.com> wrote:
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Nikolay,
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> All community members are forced to follow code style.
>>>>>>> It's harder to achieve it with dedicated suite.
>>>>>>>>>>>>>>> Why it is easier to follow code style with use of maven
>>>>>>> checkstyle
>>>>>>>>>>>>>>> plugin? Is it integrated into IDEA out of box? As I got it
>>>>>>> additional
>>>>>>>>>>>>>>> IDEA plugin is needed as well. Who will enforce everybody to
>>>>>>> install
>>>>>>>>>>>>>>> it?
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Also, as I see a common good practice today is using TC Bot
>>>>>>> visa. Visa
>>>>>>>>>>>>>>> includes result from running inspections job.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> чт, 14 февр. 2019 г. в 16:08, Nikolay Izhikov <
>>>>>>> nizhikov@apache.org>:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Ivan,
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Could you please outline the benefits you see of failing
>>>>>>> compilation and
>>>>>>>>>>>>>>>> skipping tests execution if inspections detect a problem?
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> All community members are forced to follow code style.
>>>>>>>>>>>>>>>> It's harder to achieve it with dedicated suite.
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> чт, 14 февр. 2019 г. в 15:21, Павлухин Иван <
>>>>>>> vololo100@gmail.com>:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Nikolay,
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Should the community spend TC resources for  prototype?
>>>>>>>>>>>>>>>>> Why not? I think it is not bad idea to run all tests
>>>>>>> against some
>>>>>>>>>>>>>>>>> changes into core classes. If I have a clever idea which
>>>>>>> is easy to
>>>>>>>>>>>>>>>>> test drive I can do couple of prototype-test iterations.
>>>>>>> If tests
>>>>>>>>>>>>>>>>> shows me that everything is bad then the idea was not so
>>>>>>> clever and
>>>>>>>>>>>>>>>>> easy. But if I was lucky then I should discuss the idea
>>>>>>> with other
>>>>>>>>>>>>>>>>> Igniters. I think it is the cheapest way to check the
>>>>>>> idea because the
>>>>>>>>>>>>>>>>> check is fully automated. Requiring a human feedback is
>>>>>>> much more
>>>>>>>>>>>>>>>>> expensive in my opinion.
>>>>>>>>>>>>>>>>>> But, If our code style is not convinient for every day
>>>>>>> coding for many
>>>>>>>>>>>>>>>>> contributors, should you initiate discussion to change
>>>>>>> it?
>>>>>>>>>>>>>>>>> Generally I am fine with our codestyle requirements.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Also, I would like to keep a focus on the subject. Could
>>>>>>> you please
>>>>>>>>>>>>>>>>> outline the benefits you see of failing compilation and
>>>>>>> skipping tests
>>>>>>>>>>>>>>>>> execution if inspections detect a problem?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> чт, 14 февр. 2019 г. в 14:14, Nikolay Izhikov <
>>>>>>> nizhikov@apache.org>:
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Hello, Ivan.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Requirements for a prototype code are not the same
>>>>>>> as for a patch ready
>>>>>>>>>>>>>>>>>> to merge
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> True.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> I do not see much need in writing good javadocs for
>>>>>>> prototype.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> We, as a community, can't force you to do it.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Why should I stub it to be able run any build on TC?
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Should the community spend TC resources for  prototype?
>>>>>>>>>>>>>>>>>> You always can check tests for your prototype locally.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> And when it's ready, at least from code style point of
>>>>>>> view run it on TC.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> I, personally, always try to follow project code
>>>>>>> style, even for
>>>>>>>>>>>>>>>>> prototypes.
>>>>>>>>>>>>>>>>>> But, If our code style is not convinient for every day
>>>>>>> coding for many
>>>>>>>>>>>>>>>>>> contributors, should you initiate discussion to change
>>>>>>> it?
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> ср, 13 февр. 2019 г. в 16:45, Павлухин Иван <
>>>>>>> vololo100@gmail.com>:
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Maxim,
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Oh, my poor tabs.. Joke.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> I am totally ok with currently enabled checks. But I
>>>>>>> am mostly
>>>>>>>>>>>>>>>>>>> concerned about a general approach. I would like to
>>>>>>> outline one thing.
>>>>>>>>>>>>>>>>>>> Requirements for a prototype code are not the same
>>>>>>> as for a patch
>>>>>>>>>>>>>>>>>>> ready to merge (see a little bit more in the end of
>>>>>>> that message).
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> We have a document defining code style which every
>>>>>>> contributor should
>>>>>>>>>>>>>>>>>>> follow [1]. And many points can be checked
>>>>>>> automatically. Personally,
>>>>>>>>>>>>>>>>>>> I do not see much need in writing good javadocs for
>>>>>>> prototype. Why
>>>>>>>>>>>>>>>>>>> should I stub it to be able run any build on TC?
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Also, we a have a review process which should be
>>>>>>> applied to every
>>>>>>>>>>>>>>>>>>> patch. Partially it is described in [2]. And due to
>>>>>>> this process every
>>>>>>>>>>>>>>>>>>> patch should not introduce new failures on TC. So,
>>>>>>> the patch should
>>>>>>>>>>>>>>>>>>> not be merged if inspections failed.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> P.S. Something more about prototypes and production
>>>>>>> code. There is a
>>>>>>>>>>>>>>>>>>> common bad practice in software engineering. It is
>>>>>>> turning prototypes
>>>>>>>>>>>>>>>>>>> into production code. Often it is much faster to
>>>>>>> create a prototype by
>>>>>>>>>>>>>>>>>>> price of violating some rules of writing "clean
>>>>>>> code". And often
>>>>>>>>>>>>>>>>>>> prototype after successful piloting is turned into
>>>>>>> production code.
>>>>>>>>>>>>>>>>>>> And it is very easy in practice to keep some pieces
>>>>>>> of initially
>>>>>>>>>>>>>>>>>>> "dirty" prototype code. I believe human factor plays
>>>>>>> a great role
>>>>>>>>>>>>>>>>>>> here. How should it be done right then? In my
>>>>>>> opinion good production
>>>>>>>>>>>>>>>>>>> code should be designed as "good production code"
>>>>>>> from the beginning.
>>>>>>>>>>>>>>>>>>> So, only ideas are taken from the prototype and a
>>>>>>> code is fully
>>>>>>>>>>>>>>>>>>> rewritten.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>> 
>>>>>>> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines
>>>>>>>>>>>>>>>>>>> [2]
>>>>>>>>>>>>>>>>> 
>>>>>>> https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> ср, 13 февр. 2019 г. в 15:05, Maxim Muzafarov <
>>>>>>> maxmuzaf@gmail.com>:
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Ivan,
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> As the first implementation of this addition, I'd
>>>>>>> prefer to make it
>>>>>>>>>>>>>>>>>>>> working like _Licenses Headers_ suite check. It
>>>>>>> will fail when some
>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>>> the code style checks violated. Moreover, these
>>>>>>> licenses header
>>>>>>>>>>>>>>>>> checks
>>>>>>>>>>>>>>>>>>>> can be included in the checkstyle plugin
>>>>>>> configuration.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> In general, I'd prefer to have a compilation fail
>>>>>>> error with code
>>>>>>>>>>>>>>>>>>>> style checks and after we will get a stable
>>>>>>> checkstyle suite I
>>>>>>>>>>>>>>>>> propose
>>>>>>>>>>>>>>>>>>>> to change it in a "compilation error" way. If we
>>>>>>> are talking about
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> coding style convenient for most of the community
>>>>>>> members I see no
>>>>>>>>>>>>>>>>>>>> difference with coding sketches or
>>>>>>> production-ready branches equally.
>>>>>>>>>>>>>>>>>>>> Indeed, no one will be against unused imports [or
>>>>>>> spaces instead of
>>>>>>>>>>>>>>>>>>>> tabs :-) ] in their PRs or prototypes, right? (for
>>>>>>> instance, it can
>>>>>>>>>>>>>>>>> be
>>>>>>>>>>>>>>>>>>>> automatically removed by IDE at commit phase).
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Please, note currently enabled checks are:
>>>>>>>>>>>>>>>>>>>> - list.isEmpty() instead of list.size() == 0
>>>>>>>>>>>>>>>>>>>> - unused imports
>>>>>>>>>>>>>>>>>>>> - missing @Override
>>>>>>>>>>>>>>>>>>>> - sotred modifiers checks (e.g. pulic static
>>>>>>> final ..)
>>>>>>>>>>>>>>>>>>>> - redundunt suppersion checks
>>>>>>>>>>>>>>>>>>>> - spaces insted of tabs.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Are you really what to violate these checks in
>>>>>>> your sketches? Hope
>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>> :-)
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> On Wed, 13 Feb 2019 at 10:25, Nikolay Izhikov <
>>>>>>> nizhikov@apache.org>
>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 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.weider@gmail.com:
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> I guess that was about failing build
>>>>>>> configuration with
>>>>>>>>>>>>>>>>> Checkstype,
>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>>>> compilation build itself.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> On 12 Feb 2019, at 18:03, Павлухин Иван <
>>>>>>> vololo100@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 <
>>>>>>>>>>>>>>>>> nizhikov@apache.org
>>>>>>>>>>>>>>>>>>>> :
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> Petr, we should have 1 configuration for
>>>>>>> project, may be 1
>>>>>>>>>>>>>>>>>>> configuration
>>>>>>>>>>>>>>>>>>>>>>>> per programming language.
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> пн, 11 февр. 2019 г., 11:33 Petr Ivanov
>>>>>>> mr.weider@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
>>>>>>> <
>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> --
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>>>> 
>>>>>>>>>>>> --
>>>>>>>>>>>> --
>>>>>>>>>>>> Maxim Muzafarov
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> --
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Ivan Pavlukhin
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> --
>>>>>>>> Best regards,
>>>>>>>> Ivan Pavlukhin
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> Best regards,
>>>>>>> Ivan Pavlukhin
>>>>>>> 
>>>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> --
>>>> Best Regards, Vyacheslav D.
>>>> 
>> 
>> 
>> 
>> --
>> Best Regards, Vyacheslav D.
> 
> 
> 
> -- 
> Best Regards, Vyacheslav D.


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