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.
