Vyacheslav, can you check this build please [1] if everything was ran as expected?
Also I still strictly against adding checkstyle to project build as minor issues in checkstyle should not be blocker for test run. [1] https://ci.ignite.apache.org/viewLog.html?buildId=3678000&buildTypeId=IgniteTests24Java8_CheckCodeStyle&tab=artifacts&branch_IgniteTests24Java8=%3Cdefault%3E# > On 23 Apr 2019, at 11:59, Petr Ivanov <mr.wei...@gmail.com> wrote: > > I'll check it. > > > Also, please pass TC build for review next time and do not add to Run All > without it. > > Thanks! > > >> On 23 Apr 2019, at 11:53, Vyacheslav Daradur <daradu...@gmail.com> wrote: >> >> This is quite strange error, in case of "install" phase it'd be better >> just add "checkstyle" profile to "Build" [1] configuration. >> >> [1] >> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite >> >> On Tue, Apr 23, 2019 at 11:43 AM Petr Ivanov <mr.wei...@gmail.com> wrote: >>> >>> 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 >>> >>> >>> On 23 Apr 2019, at 00:25, Vyacheslav Daradur <daradu...@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 <daradu...@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 <maxmu...@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 <daradu...@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 <maxmu...@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 <dpav...@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, Павлухин Иван <vololo...@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, Павлухин Иван <vololo...@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 <nizhi...@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 <maxmu...@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, Павлухин Иван <vololo...@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 <maxmu...@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 Павлухин Иван <vololo...@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 <maxmu...@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, Павлухин Иван < >>> >>> vololo...@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 < >>> >>> nizhi...@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, Павлухин Иван < >>> >>> vololo...@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 < >>> >>> nizhi...@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, Павлухин Иван < >>> >>> vololo...@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 < >>> >>> maxmu...@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 < >>> >>> nizhi...@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.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 >>> >>> >>> >>> >>> >>> >>> -- >>> 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. >>> >>> >> >> >> -- >> Best Regards, Vyacheslav D. >
