From dev-return-44667-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Mon Feb 11 08:02:24 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 [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id B4DD6180648 for ; Mon, 11 Feb 2019 09:02:23 +0100 (CET) Received: (qmail 53990 invoked by uid 500); 11 Feb 2019 08:02:22 -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 53978 invoked by uid 99); 11 Feb 2019 08:02:22 -0000 Received: from mail-relay.apache.org (HELO mailrelay2-lw-us.apache.org) (207.244.88.137) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 11 Feb 2019 08:02:22 +0000 Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by mailrelay2-lw-us.apache.org (ASF Mail Server at mailrelay2-lw-us.apache.org) with ESMTPSA id 4A07E2752 for ; Mon, 11 Feb 2019 08:02:21 +0000 (UTC) Received: by mail-lf1-f47.google.com with SMTP id l10so6958658lfh.9 for ; Mon, 11 Feb 2019 00:02:21 -0800 (PST) X-Gm-Message-State: AHQUAuYNEJwix1kduft3EhI7Vvg/5dRWem/x4+5zU+ybhhv9RvPfp+/1 9dD43WfV95Y5PVjekETBIr0XFP3on8e70oV61Nk= X-Google-Smtp-Source: AHgI3IaISV8gTvnXqYoV/jqBC8i8Heb15VnBSzQp4kS4M6hIwBzq7VaESUpkMA0VbPrUGuOFMM00jsh+PBfg9rFLvYA= X-Received: by 2002:a19:9945:: with SMTP id b66mr21334738lfe.55.1549872140091; Mon, 11 Feb 2019 00:02:20 -0800 (PST) MIME-Version: 1.0 References: <1549753115822-0.post@n4.nabble.com> In-Reply-To: From: Nikolay Izhikov Date: Mon, 11 Feb 2019 11:02:08 +0300 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Code inspection To: dev Content-Type: multipart/alternative; boundary="0000000000005b87eb058199ba16" --0000000000005b87eb058199ba16 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. =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 : > Hi, > > I also think that some warning from 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, oig= natenko : > > > > Hi Maxim, > > > > I believe that whatever style checks we establish at Teamcity, we bette= r > > take care of making it easy for developers to find and fix violations i= n > > 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 inspection= s > to > > checkstyle / maven. > > > > This is because I am very disappointed observing how it stays broken fo= r > 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 t= o > > break for some obscure reason, this is so much better than what I obser= ve > > 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 th= at > > developers could easily assess and discuss style settings and keep trac= k > 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 onl= y > > > 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 styl= e > > > 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. W= e > > > 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=3DIgniteTests24Jav= a8_InspectionsCore&branch_IgniteTests24Java8=3D%3Cdefault%3E&tab=3DbuildTyp= eStatusDiv > > > [2] https://youtrack.jetbrains.com/issue/TW-58504 > > > [3] > https://github.com/apache/ignite/blob/master/modules/core/src/test/java/o= rg/apache/ignite/internal/processors/cache/IgniteCachePutAllRestartTest.jav= a#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 > --0000000000005b87eb058199ba16--