From dev-return-45753-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Wed Apr 17 18:23:09 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 915C6180672 for ; Wed, 17 Apr 2019 20:23:08 +0200 (CEST) Received: (qmail 16307 invoked by uid 500); 17 Apr 2019 18:23:07 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 16294 invoked by uid 99); 17 Apr 2019 18:23:06 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Apr 2019 18:23:06 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id D0216C2439 for ; Wed, 17 Apr 2019 18:23:05 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.966 X-Spam-Level: ** X-Spam-Status: No, score=2.966 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_REPLY=1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URI_HEX=1.313, URI_TRY_3LD=0.853] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id mxoeZA-sRb4C for ; Wed, 17 Apr 2019 18:23:02 +0000 (UTC) Received: from mail-pg1-f195.google.com (mail-pg1-f195.google.com [209.85.215.195]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 9942960D02 for ; Wed, 17 Apr 2019 18:23:01 +0000 (UTC) Received: by mail-pg1-f195.google.com with SMTP id j26so12374522pgl.5 for ; Wed, 17 Apr 2019 11:23:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :content-transfer-encoding; bh=skNXxM/Ob0M28w3MaLF3kcHS0hnESQhkWVGrwOAv4wc=; b=MQPXbusdEHeYOKe0+0UtiOY4qvF6UDvMr5D9E8IHaRa9PBpNB+NXfqG5xxwsVVIYbZ jhlMYa/E6k/PHuRufHuHxqEr9ALD/HmX5eKNQMtawiBgFIteuogg3HoZ9TbHnvOZSMyU TU/JQ3PlWbIoJbRvwghXQHUJxlcS5mHj4bGwp8kH7AOggpkDV3csCI9464DOhYG1h5eV yIeSGuNXhb1k5uWdD1J6nO0ZSNq9fSKDCSuUM7V1yUTw0AU+PlVjdpJfd4KivVWbFc2i +5pUjL08JRQ4Y5UafKkxIBFwbt9XnmCN5aHBten7Kd8Zk1OSXTn5zWaRW/Ugh/3NLOFr iExw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:content-transfer-encoding; bh=skNXxM/Ob0M28w3MaLF3kcHS0hnESQhkWVGrwOAv4wc=; b=jlPR07zBDe3l8pvmnCCPoLjGxaP2Tt0IEJ/b02qmUbnQd/QMCZBvE3ZAA7PCOMqkkF fk/0sSMnHAy8WkiP+5FY2oV4j7TIGUIYags1VMNFcffQIjkWgcvunNMUfPGmAb9UCahI 66jNc2ZiDwVlLgiwRLCYSP4GQy9C0ib+iG6+/54Ag3CaSFdBKaviMhzFJKjDPfO1Yxs/ FBcHL991Mdsw2BFmpMPnXYrnLjMP/H7NVc1qDbFu/WHTsCNBxwG2GnXrGeNPVOwuKmqN KEfrUkI5hWaHD7PaNKNUX7RgQzcDJgeHoh9LHNcA2vqrdjXkqIYIuB7/ovYbKd59Ff2w 5Hrw== X-Gm-Message-State: APjAAAXelJ23UQISAR/D7OKd6QdMCfGYjNchkFnNjgY7rm/SsQLmAY3v l7BdDyRCpO+XuWYuNcEv5yB4A0IpBPE9+lGxNIATHi+ApKQ= X-Google-Smtp-Source: APXvYqxU2RBEUUkYY9631YG1QCGn9Vu4ijsoU/XzqduRKaN4BiIL86sYxOdx+nnWYwm1mNcq279s3xXnjueHOpZaUNI= X-Received: by 2002:aa7:8ac8:: with SMTP id b8mr91057339pfd.234.1555525379586; Wed, 17 Apr 2019 11:22:59 -0700 (PDT) MIME-Version: 1.0 References: <1549753115822-0.post@n4.nabble.com> <6D73E5EC-A96B-4194-AD06-FB765466A3BA@gmail.com> In-Reply-To: From: Vyacheslav Daradur Date: Wed, 17 Apr 2019 21:22:49 +0300 Message-ID: Subject: Re: Code inspection To: dev@ignite.apache.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 wrote= : > > Igniters, > > The issue [1] with enabled maven-checkstyle-plugin is ready for the revie= w. > 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 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 > > > > =D0=BF=D1=82, 15 =D0=BC=D0=B0=D1=80. 2019 =D0=B3. =D0=B2 12:04, =D0=9F= =D0=B0=D0=B2=D0=BB=D1=83=D1=85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD : > > > > > I agree to gather some votes according to Maxim's proposal. > > > > > > Personally, I will not put my vote here. Both options will work for m= e > > > 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 t= o > > > another easily in future. Let's do our best to be flexible. > > > > > > =D0=BF=D1=82, 15 =D0=BC=D0=B0=D1=80. 2019 =D0=B3. =D0=B2 12:04, =D0= =9F=D0=B0=D0=B2=D0=BB=D1=83=D1=85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD : > > > > > > > > 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 task= s > > > > 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 d= oes > > > > 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 becau= se > > > > 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 c= an > > > > request Checkstyle visa. Committers should check formalities. > > > > > > > > =D0=BF=D1=82, 15 =D0=BC=D0=B0=D1=80. 2019 =D0=B3. =D0=B2 10:29, Nik= olay Izhikov : > > > > > > > > > > +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 branc= h can > > > set this variable and write some nasty code before push :) > > > > > > > > > > =D0=BF=D1=82, 15 =D0=BC=D0=B0=D1=80=D1=82=D0=B0 2019 =D0=B3., 9:4= 0 Maxim Muzafarov : > > > > >> > > > > >> Ivan, > > > > >> > > > > >> > 1. I can write code and execute tests successfully even if the= re 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 re= d 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 style= s > > > > >> issues right before getting the final TC results. Right? It's do= able > > > > >> you can disable checkstyle in your local branch and revet it bac= k 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 al= l > > > > >> 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 yo= ur 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, someth= ing > > > > >> else in future) > > > > >> > > > > >> + a bit comfortable way of TC tests execution for local\prototyp= ed PRs > > > > >> (it's a matter of taste) > > > > >> + build the project and execute test suites a bit earlier (check= style > > > > >> on the separate suite does not affect other suites) > > > > >> > > > > >> 2) (code style will be broken less often) Run checkstyle on proj= ect > > > 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 bran= ch > > > > >> starts to fail > > > > >> + the single place for code checks on maven code validation stag= e > > > > >> (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 Igni= te > > > needs. > > > > >> > > > > >> --- > > > > >> > > > > >> Personally, I'd like to choose the 2) option. > > > > >> > > > > >> The JIRA [2] and PR [3] with the checkstyle enabled plugin is re= ady > > > > >> 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, =D0=9F=D0=B0=D0=B2=D0=BB=D1=83=D1= =85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD > > > wrote: > > > > >> > > > > > >> > Maxim, > > > > >> > > > > > >> > From use cases described by you I use only 1 and 2. And actual= ly I > > > > >> > think that we can concentrate on 1 and forget about others for= now. > > > > >> > But please address my worries from previous letter: > > > > >> > =3D=3D=3D=3DQuoted text=3D=3D=3D=3D > > > > >> > 1. I can write code and execute tests successfully even if the= re are > > > > >> > some style problems. I can imagine that a style error can aris= e > > > > >> > occasionally. And instead of getting test results after severa= l > > > hours > > > > >> > I will get a build failure without any tests run. I will simpl= y 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 almos= t > > > > >> > negligible. But it can grow if more checks are involved. > > > > >> > =3D=3D=3D=3DEnd of quoted text=3D=3D=3D=3D > > > > >> > > > > > >> > Some clarifications. 1 is about running from IDEA first. 2 is > > > related > > > > >> > to opinions that we should involve much more checks, e.g. usin= g > > > > >> > abbreviations. > > > > >> > > > > > >> > =D1=87=D1=82, 7 =D0=BC=D0=B0=D1=80. 2019 =D0=B3. =D0=B2 10:36,= Maxim Muzafarov : > > > > >> > > > > > > >> > > 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 section, so the user has no = chance > > > in common cases to disable it via command line (correct me if I'm wro= ng). > > > In the PR [1] I've moved checkstyle configuration to the separate pro= file. > > > I've set activation checkstyle profile if -DskipTests specified, so i= t will > > > run with the basic build configuration. It can also be disabled local= ly 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 b= e > > > violated. If we will use the TC.Bot approach someone will merge the b= ranch > > > without running TC.Bot on it, but no one will merge the branch with c= ompile > > > 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 ins= tall > > > -DskipTests` will activate the profile. > > > > >> > > > > > > >> > > 4. The checkstyle profile can be disabled explicitly on TC b= y > > > specifying -P !checkstyle option. A don't see any use cases of it, bu= t 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 =D0=9F=D0=B0=D0=B2=D0=BB=D1=83= =D1=85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD > > > wrote: > > > > >> > >> > > > > >> > >> Maxim, > > > > >> > >> > > > > >> > >> I like an idea of being IDE agnostic. I am ok with currentl= y > > > enabled > > > > >> > >> checks, they are a must-have in my opinion (even for protot= ypes). > > > > >> > >> > > > > >> > >> But I am still do not like an idea of preventing tests exec= ution > > > 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 a= rise > > > > >> > >> occasionally. And instead of getting test results after sev= eral > > > hours > > > > >> > >> I will get a build failure without any tests run. I will si= mply > > > 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 che= ck. > > > > >> > >> 2. Style check takes some time. With simple checks it is al= most > > > > >> > >> negligible. But it can grow if more checks are involved. > > > > >> > >> > > > > >> > >> On the bright side I found nice integration of Checkstyle p= lugin > > > with > > > > >> > >> IDEA commit dialog. There is a checkbox "Scan with Checksty= le" > > > which I > > > > >> > >> think is quite useful. > > > > >> > >> > > > > >> > >> =D0=BF=D0=BD, 4 =D0=BC=D0=B0=D1=80. 2019 =D0=B3. =D0=B2 15:= 00, Maxim Muzafarov > > >: > > > > >> > >> > > > > > >> > >> > Ivan, > > > > >> > >> > > > > > >> > >> > I like that Jetbrains inspections are integrated with IDE= and > > > TC out > > > > >> > >> > of the box, but currently, they are working not well enou= gh on > > > TC. > > > > >> > >> > Actually, they are not checking our source code at all. > > > > >> > >> > > > > > >> > >> > Let's try a bit another approach and try to be IDE-agnost= ic > > > with code > > > > >> > >> > style checking. I've checked popular java projects: hadoo= p, > > > kafka, > > > > >> > >> > spark, hive, netty. All of them are using > > > maven-checkstyle-plugin in > > > > >> > >> > their section by default, so why don't we? It sou= nds > > > > >> > >> > 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 RedundantSuppress= ion > > > and > > > > >> > >> > SizeReplaceableByIsEmpty (all code style checks [5]). Pro= pose > > > to merge > > > > >> > >> > without them. > > > > >> > >> > 2) Checkstyle plugin has it's own maven profile and enabl= ed 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= =3D16771200&page=3Dcom.atlassian.jira.plugin.system.issuetabpanels%3Acommen= t-tabpanel#comment-16771200 > > > > >> > >> > [5] http://checkstyle.sourceforge.net/checks.html > > > > >> > >> > > > > > >> > >> > On Thu, 14 Feb 2019 at 16:21, =D0=9F=D0=B0=D0=B2=D0=BB=D1= =83=D1=85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD < > > > 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 everybo= dy to > > > install > > > > >> > >> > > it? > > > > >> > >> > > > > > > >> > >> > > Also, as I see a common good practice today is using TC= Bot > > > visa. Visa > > > > >> > >> > > includes result from running inspections job. > > > > >> > >> > > > > > > >> > >> > > =D1=87=D1=82, 14 =D1=84=D0=B5=D0=B2=D1=80. 2019 =D0=B3.= =D0=B2 16:08, Nikolay Izhikov < > > > nizhikov@apache.org>: > > > > >> > >> > > > > > > > >> > >> > > > Ivan, > > > > >> > >> > > > > > > > >> > >> > > > > Could you please outline the benefits you see of fa= iling > > > compilation and > > > > >> > >> > > > skipping tests execution if inspections detect a prob= lem? > > > > >> > >> > > > > > > > >> > >> > > > All community members are forced to follow code style= . > > > > >> > >> > > > It's harder to achieve it with dedicated suite. > > > > >> > >> > > > > > > > >> > >> > > > > > > > >> > >> > > > =D1=87=D1=82, 14 =D1=84=D0=B5=D0=B2=D1=80. 2019 =D0= =B3. =D0=B2 15:21, =D0=9F=D0=B0=D0=B2=D0=BB=D1=83=D1=85=D0=B8=D0=BD =D0=98= =D0=B2=D0=B0=D0=BD < > > > vololo100@gmail.com>: > > > > >> > >> > > > > > > > >> > >> > > > > Nikolay, > > > > >> > >> > > > > > > > > >> > >> > > > > > Should the community spend TC resources for prot= otype? > > > > >> > >> > > > > Why not? I think it is not bad idea to run all test= s > > > 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 iterat= ions. > > > If tests > > > > >> > >> > > > > shows me that everything is bad then the idea was n= ot 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 t= he > > > idea because the > > > > >> > >> > > > > check is fully automated. Requiring a human feedbac= k is > > > much more > > > > >> > >> > > > > expensive in my opinion. > > > > >> > >> > > > > > But, If our code style is not convinient for ever= y day > > > coding for many > > > > >> > >> > > > > contributors, should you initiate discussion to cha= nge > > > 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? > > > > >> > >> > > > > > > > > >> > >> > > > > =D1=87=D1=82, 14 =D1=84=D0=B5=D0=B2=D1=80. 2019 =D0= =B3. =D0=B2 14:14, Nikolay Izhikov < > > > nizhikov@apache.org>: > > > > >> > >> > > > > > > > > > >> > >> > > > > > Hello, Ivan. > > > > >> > >> > > > > > > > > > >> > >> > > > > > > Requirements for a prototype code are not the s= ame > > > 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 o= n TC? > > > > >> > >> > > > > > > > > > >> > >> > > > > > Should the community spend TC resources for prot= otype? > > > > >> > >> > > > > > You always can check tests for your prototype loc= ally. > > > > >> > >> > > > > > > > > > >> > >> > > > > > And when it's ready, at least from code style poi= nt 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 ever= y day > > > coding for many > > > > >> > >> > > > > > contributors, should you initiate discussion to c= hange > > > it? > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > =D1=81=D1=80, 13 =D1=84=D0=B5=D0=B2=D1=80. 2019 = =D0=B3. =D0=B2 16:45, =D0=9F=D0=B0=D0=B2=D0=BB=D1=83=D1=85=D0=B8=D0=BD =D0= =98=D0=B2=D0=B0=D0=BD < > > > 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 lik= e to > > > outline one thing. > > > > >> > >> > > > > > > Requirements for a prototype code are not the s= ame > > > as for a patch > > > > >> > >> > > > > > > ready to merge (see a little bit more in the en= d of > > > that message). > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > We have a document defining code style which ev= ery > > > 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 b= e > > > applied to every > > > > >> > >> > > > > > > patch. Partially it is described in [2]. And du= e 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 produc= tion > > > code. There is a > > > > >> > >> > > > > > > common bad practice in software engineering. It= is > > > turning prototypes > > > > >> > >> > > > > > > into production code. Often it is much faster t= o > > > create a prototype by > > > > >> > >> > > > > > > price of violating some rules of writing "clean > > > code". And often > > > > >> > >> > > > > > > prototype after successful piloting is turned i= nto > > > production code. > > > > >> > >> > > > > > > And it is very easy in practice to keep some pi= eces > > > 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 cod= e" > > > 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 > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > =D1=81=D1=80, 13 =D1=84=D0=B5=D0=B2=D1=80. 2019= =D0=B3. =D0=B2 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, the= se > > > 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 commu= nity > > > 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() =3D= =3D 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 i= n > > > 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 fa= iling > > > *compilation* > > > > >> > >> > > > > task. > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > I think one should use project code style f= or > > > everyday coding, not > > > > >> > >> > > > > > > only for > > > > >> > >> > > > > > > > > ready-to-merge PRs. > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > If we cant use code style for everyday codi= ng, > > > we should change the > > > > >> > >> > > > > > > > > codestyle. > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > What do you think? > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > =D1=81=D1=80, 13 =D1=84=D0=B5=D0=B2=D1=80. = 2019 =D0=B3., 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, =D0=9F=D0=B0= =D0=B2=D0=BB=D1=83=D1=85=D0=B8=D0=BD =D0=98=D0=B2=D0=B0=D0=BD < > > > vololo100@gmail.com> > > > > >> > >> > > > > > > wrote: > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > Folks, > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > Are you going to fail job compiling Ign= ite > > > sources [1] if some > > > > >> > >> > > > > > > > > > > inspection found a problem? Can we avoi= d it? > > > It is quite common > > > > >> > >> > > > > > > > > > > pattern to start some feature implement= ation > > > with making a > > > > >> > >> > > > > sketch > > > > >> > >> > > > > > > and > > > > >> > >> > > > > > > > > > > running tests against it. I found it > > > convenient to skip some > > > > >> > >> > > > > style > > > > >> > >> > > > > > > > > > > requirements for such sketches (e.g. we= ll > > > formed javadocs). > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > [1] > > > > >> > >> > > > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > https://ci.ignite.apache.org/viewType.html?buildTypeId=3DIgniteTests2= 4Java8_BuildApacheIgnite > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > =D0=BF=D0=BD, 11 =D1=84=D0=B5=D0=B2=D1= =80. 2019 =D0=B3. =D0=B2 11:38, Nikolay > > > Izhikov < > > > > >> > >> > > > > nizhikov@apache.org > > > > >> > >> > > > > > > >: > > > > >> > >> > > > > > > > > > >> > > > > >> > >> > > > > > > > > > >> Petr, we should have 1 configuration f= or > > > project, may be 1 > > > > >> > >> > > > > > > configuration > > > > >> > >> > > > > > > > > > >> per programming language. > > > > >> > >> > > > > > > > > > >> > > > > >> > >> > > > > > > > > > >> =D0=BF=D0=BD, 11 =D1=84=D0=B5=D0=B2=D1= =80. 2019 =D0=B3., 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 Iz= hikov > > > < > > > > >> > >> > > > > nizhikov@apache.org> > > > > >> > >> > > > > > > > > > wrote: > > > > >> > >> > > > > > > > > > >>>> > > > > >> > >> > > > > > > > > > >>>> Hello, Petr. > > > > >> > >> > > > > > > > > > >>>> > > > > >> > >> > > > > > > > > > >>>> Are you saying that we have not sing= le > > > build task? And each > > > > >> > >> > > > > > > module > > > > >> > >> > > > > > > > > > builds > > > > >> > >> > > > > > > > > > >>>> when it required? If yes, then I pro= pose > > > to create a task > > > > >> > >> > > > > like > > > > >> > >> > > > > > > > > > "Licence > > > > >> > >> > > > > > > > > > >>>> check" which will be run for every p= atch. > > > > >> > >> > > > > > > > > > >>>> > > > > >> > >> > > > > > > > > > >>>> My point is that violation of codest= yle > > > should be treated as > > > > >> > >> > > > > > > hard as > > > > >> > >> > > > > > > > > > >>>> compile error. > > > > >> > >> > > > > > > > > > >>>> > > > > >> > >> > > > > > > > > > >>>> =D0=BF=D0=BD, 11 =D1=84=D0=B5=D0=B2= =D1=80. 2019 =D0=B3., 11:16 Petr Ivanov > > > mr.weider@gmail.com > > > > >> > >> > > > > : > > > > >> > >> > > > > > > > > > >>>> > > > > >> > >> > > > > > > > > > >>>>> Is build configuration Inspections > > > [Core] meant to > > > > >> > >> > > > > transform > > > > >> > >> > > > > > > into > > > > >> > >> > > > > > > > > > single > > > > >> > >> > > > > > > > > > >>>>> all-modules check build configurati= on > > > (without module > > > > >> > >> > > > > > > subdivision)? > > > > >> > >> > > > > > > > > > >>>>> > > > > >> > >> > > > > > > > > > >>>>> > > > > >> > >> > > > > > > > > > >>>>>> On 11 Feb 2019, at 11:02, Nikolay > > > Izhikov < > > > > >> > >> > > > > > > nizhikov@apache.org> > > > > >> > >> > > > > > > > > > wrote: > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> Hello, Maxim. > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> +1 from me for migrating to checks= tyle. > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> Oleg, there is plugin for IDEA wit= h > > > 2mln downloads - > > > > >> > >> > > > > > > > > > >>>>>> > > > https://plugins.jetbrains.com/plugin/1065-checkstyle-idea > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> I propose do the following: > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> 1. Migrate current checks to check= style. > > > > >> > >> > > > > > > > > > >>>>>> 2. Apply checks to all Ignite modu= les. > > > Currently, only > > > > >> > >> > > > > core > > > > >> > >> > > > > > > module > > > > >> > >> > > > > > > > > > are > > > > >> > >> > > > > > > > > > >>>>>> checked. > > > > >> > >> > > > > > > > > > >>>>>> I will review and commit this patc= h, or > > > do it by my own. > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> 3. Include code style checks to "B= uild > > > Apache Ignite" > > > > >> > >> > > > > suite. > > > > >> > >> > > > > > > Ignite > > > > >> > >> > > > > > > > > > has > > > > >> > >> > > > > > > > > > >>>>> to > > > > >> > >> > > > > > > > > > >>>>>> fail to build if patch violates > > > codestyle. > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>> =D0=B2=D1=81, 10 =D1=84=D0=B5=D0= =B2=D1=80. 2019 =D0=B3. =D0=B2 07:54, =D0=9F=D0=B0=D0=B2=D0=BB=D1=83=D1=85= =D0=B8=D0=BD > > > =D0=98=D0=B2=D0=B0=D0=BD < > > > > >> > >> > > > > > > vololo100@gmail.com>: > > > > >> > >> > > > > > > > > > >>>>>> > > > > >> > >> > > > > > > > > > >>>>>>> Hi, > > > > >> > >> > > > > > > > > > >>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>> I also think that some warning fr= om > > > IDEA that some code > > > > >> > >> > > > > > > style rule > > > > >> > >> > > > > > > > > > is > > > > >> > >> > > > > > > > > > >>>>>>> violated is a must-have. > > > > >> > >> > > > > > > > > > >>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>> =D0=B2=D1=81, 10 =D1=84=D0=B5=D0= =B2=D1=80. 2019 =D0=B3. =D0=B2 01:58, > > > oignatenko < > > > > >> > >> > > > > > > oignatenko@gridgain.com > > > > >> > >> > > > > > > > > > >: > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>> Hi Maxim, > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>> I believe that whatever style ch= ecks > > > we establish at > > > > >> > >> > > > > > > Teamcity, we > > > > >> > >> > > > > > > > > > >>>>> better > > > > >> > >> > > > > > > > > > >>>>>>>> take care of making it easy for > > > developers to find and > > > > >> > >> > > > > fix > > > > >> > >> > > > > > > > > > violations > > > > >> > >> > > > > > > > > > >>>>> in > > > > >> > >> > > > > > > > > > >>>>>>>> their typical dev environment (f= or > > > 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 whe= n > > > (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 wi= th my > > > experience > > > > >> > >> > > > > regarding > > > > >> > >> > > > > > > > > > checkstyle > > > > >> > >> > > > > > > > > > >>>>>>> based > > > > >> > >> > > > > > > > > > >>>>>>>> inspections. These just work and= you > > > just never fear > > > > >> > >> > > > > that > > > > >> > >> > > > > > > it is > > > > >> > >> > > > > > > > > > going > > > > >> > >> > > > > > > > > > >>>>> to > > > > >> > >> > > > > > > > > > >>>>>>>> break for some obscure reason, t= his > > > is so much better > > > > >> > >> > > > > than > > > > >> > >> > > > > > > what I > > > > >> > >> > > > > > > > > > >>>>> observe > > > > >> > >> > > > > > > > > > >>>>>>>> now. > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>> One suggestion in case if we pic= k > > > checkstyle - I > > > > >> > >> > > > > recommend > > > > >> > >> > > > > > > keeping > > > > >> > >> > > > > > > > > > >>> its > > > > >> > >> > > > > > > > > > >>>>>>>> config file somewhere in the pro= ject > > > under version > > > > >> > >> > > > > control. > > > > >> > >> > > > > > > I > > > > >> > >> > > > > > > > > > used to > > > > >> > >> > > > > > > > > > >>>>>>>> maintain such a shared style con= fig > > > 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 a= nd > > > discuss style > > > > >> > >> > > > > settings > > > > >> > >> > > > > > > and keep > > > > >> > >> > > > > > > > > > >>>>> track > > > > >> > >> > > > > > > > > > >>>>>>> of > > > > >> > >> > > > > > > > > > >>>>>>>> changes in these. (note how Kafk= a > > > 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]. Cu= rrent > > > 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 Ignit= e > > > 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 l= ine > > > > >> > >> > > > > > > > > > >>>>>>>>> - 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=3DIgniteTests2= 4Java8_InspectionsCore&branch_IgniteTests24Java8=3D%3Cdefault%3E&tab=3Dbuil= dTypeStatusDiv > > > > >> > >> > > > > > > > > > >>>>>>>>> [2] > > > https://youtrack.jetbrains.com/issue/TW-58504 > > > > >> > >> > > > > > > > > > >>>>>>>>> [3] > > > > >> > >> > > > > > > > > > >>>>>>> > > > > >> > >> > > > > > > > > > >>>>> > > > > >> > >> > > > > > > > > > >>> > > > > >> > >> > > > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > https://github.com/apache/ignite/blob/master/modules/core/src/test/ja= va/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/checksty= le > > > > >> > >> > > > > > > > > > >>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> On Fri, 21 Dec 2018 at 16:03, P= etr > > > Ivanov < > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> mr.weider@ > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> > wrote: > > > > >> > >> > > > > > > > > > >>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>> It seems there is bug in lates= t > > > 2018.2 TeamCity > > > > >> > >> > > > > > > > > > >>>>>>>>>> Bug is filed [1] > > > > >> > >> > > > > > > > > > >>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>> [1] > > > https://youtrack.jetbrains.com/issue/TW-58504 > > > > >> > >> > > > > > > > > > >>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>>> On 19 Dec 2018, at 11:31, Pet= r > > > Ivanov < > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> mr.weider@ > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> > wrote: > > > > >> > >> > > > > > > > > > >>>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>>> Investigating problem, stand = by. > > > > >> > >> > > > > > > > > > >>>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>>>> On 18 Dec 2018, at 19:41, Dm= itriy > > > Pavlov < > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> dpavlov@ > > > > >> > >> > > > > > > > > > >>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>> > wrote: > > > > >> > >> > > > > > > > > > >>>>>>>>>>>> > > > > >> > >> > > > > > > > > > >>>>>>>>>>>> Both patches were applied. M= axim, > > > 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 > > > > > > > --=20 Best Regards, Vyacheslav D.