ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Muzafarov <maxmu...@gmail.com>
Subject Re: Code inspection
Date Thu, 07 Mar 2019 07:36:47 GMT
Ivan,

Let's take a look at all the options we have (ordered by the frequency of
use):

1. Check ready for merge branches (main case)
2. Run tests on TC without checkstyle (prototyping branches)
3. Local project build
4. Quick build without any additional actions on TC

In the other projects (kafka, netty etc.) which I've checked the checkstyle
plugin is used in the <build> section, so the user has no chance in common
cases to disable it via command line (correct me if I'm wrong). In the PR
[1] I've moved checkstyle configuration to the separate profile. I've set
activation checkstyle profile if -DskipTests specified, so it will run with
the basic build configuration. It can also be disabled locally if we really
need it.

Back to our use cases:

1. For checking the ready to merge branches we will fail the ~Build Apache
Ignite~ suite, so no configured checkstyle rules will be violated. If we
will use the TC.Bot approach someone will merge the branch without running
TC.Bot on it, but no one will merge the branch with compile errors.

2. For the prototyping branches, you can turn off checkstyle in your local
PR by removing activation configuration. It's ok as these type of branches
will never be merged to the master.

3. From my point, local builds should be always run with the checkstyle
enabled profile. The common build action as `mvn clean install -DskipTests`
will activate the profile.

4. The checkstyle profile can be disabled explicitly on TC by specifying -P
!checkstyle option. A don't see any use cases of it, but it's completely
doable.

Please, correct me if I've missed something.

I propose to merge PR [1] as it is, with the configured set of rules.

[1] https://github.com/apache/ignite/pull/6119

On Tue, 5 Mar 2019 at 19:02 Павлухин Иван <vololo100@gmail.com> wrote:

> Maxim,
>
> I like an idea of being IDE agnostic. I am ok with currently enabled
> checks, they are a must-have in my opinion (even for prototypes).
>
> But I am still do not like an idea of preventing tests execution if
> style check finds a problem. I checked out PR, installed a plugin and
> tried it out. Here are my concerns:
> 1. I can write code and execute tests successfully even if there are
> some style problems. I can imagine that a style error can arise
> occasionally. And instead of getting test results after several hours
> I will get a build failure without any tests run. I will simply lose
> my time. But if the tests are allowed to proceed I will get a red flag
> from code style check, fix those issues and rerun style check.
> 2. Style check takes some time. With simple checks it is almost
> negligible. But it can grow if more checks are involved.
>
> On the bright side I found nice integration of Checkstyle plugin with
> IDEA commit dialog. There is a checkbox "Scan with Checkstyle" which I
> think is quite useful.
>
> пн, 4 мар. 2019 г. в 15:00, Maxim Muzafarov <maxmuzaf@gmail.com>:
> >
> > Ivan,
> >
> > I like that Jetbrains inspections are integrated with IDE and TC out
> > of the box, but currently, they are working not well enough on TC.
> > Actually, they are not checking our source code at all.
> >
> > Let's try a bit another approach and try to be IDE-agnostic with code
> > style checking. I've checked popular java projects: hadoop, kafka,
> > spark, hive, netty. All of them are using maven-checkstyle-plugin in
> > their <build> section by default, so why don't we? It sounds
> > reasonable for me at least to try so.
> >
> > Can you take a look at my changes below?
> >
> >
> > Igniters,
> >
> > PR [2] has been prepared. All the details I've mentioned in my comment
> > in JIRA [4].
> > Can anyone take a look at my changes?
> >
> > JIRA: [1]
> > PR: [2]
> > Upsource: [3]
> >
> > Questions to discuss:
> > 1) There is no analogue for inspections RedundantSuppression and
> > SizeReplaceableByIsEmpty (all code style checks [5]). Propose to merge
> > without them.
> > 2) Checkstyle plugin has it's own maven profile and enabled by
> > default. It can be turned off for prototype branches.
> > 3) I've removed the inspections configuration for the TC suite and
> > propose to disable it as not working.
> >
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-11277
> > [2] https://github.com/apache/ignite/pull/6119
> > [3] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1018
> > [4]
> https://issues.apache.org/jira/browse/IGNITE-11277?focusedCommentId=16771200&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16771200
> > [5] http://checkstyle.sourceforge.net/checks.html
> >
> > On Thu, 14 Feb 2019 at 16:21, Павлухин Иван <vololo100@gmail.com>
wrote:
> > >
> > > Nikolay,
> > >
> > > > All community members are forced to follow code style. It's harder
> to achieve it with dedicated suite.
> > > Why it is easier to follow code style with use of maven checkstyle
> > > plugin? Is it integrated into IDEA out of box? As I got it additional
> > > IDEA plugin is needed as well. Who will enforce everybody to install
> > > it?
> > >
> > > Also, as I see a common good practice today is using TC Bot visa. Visa
> > > includes result from running inspections job.
> > >
> > > чт, 14 февр. 2019 г. в 16:08, Nikolay Izhikov <nizhikov@apache.org>:
> > > >
> > > > Ivan,
> > > >
> > > > > Could you please outline the benefits you see of failing
> compilation and
> > > > skipping tests execution if inspections detect a problem?
> > > >
> > > > All community members are forced to follow code style.
> > > > It's harder to achieve it with dedicated suite.
> > > >
> > > >
> > > > чт, 14 февр. 2019 г. в 15:21, Павлухин Иван <vololo100@gmail.com>:
> > > >
> > > > > Nikolay,
> > > > >
> > > > > > Should the community spend TC resources for  prototype?
> > > > > Why not? I think it is not bad idea to run all tests against some
> > > > > changes into core classes. If I have a clever idea which is easy
to
> > > > > test drive I can do couple of prototype-test iterations. If tests
> > > > > shows me that everything is bad then the idea was not so clever and
> > > > > easy. But if I was lucky then I should discuss the idea with other
> > > > > Igniters. I think it is the cheapest way to check the idea because
> the
> > > > > check is fully automated. Requiring a human feedback is much more
> > > > > expensive in my opinion.
> > > > > > But, If our code style is not convinient for every day coding
> for many
> > > > > contributors, should you initiate discussion to change it?
> > > > > Generally I am fine with our codestyle requirements.
> > > > >
> > > > > Also, I would like to keep a focus on the subject. Could you please
> > > > > outline the benefits you see of failing compilation and skipping
> tests
> > > > > execution if inspections detect a problem?
> > > > >
> > > > > чт, 14 февр. 2019 г. в 14:14, Nikolay Izhikov <nizhikov@apache.org
> >:
> > > > > >
> > > > > > Hello, Ivan.
> > > > > >
> > > > > > > Requirements for a prototype code are not the same as for
a
> patch ready
> > > > > > to merge
> > > > > >
> > > > > > True.
> > > > > >
> > > > > > > I do not see much need in writing good javadocs for prototype.
> > > > > >
> > > > > > We, as a community, can't force you to do it.
> > > > > >
> > > > > > > Why should I stub it to be able run any build on TC?
> > > > > >
> > > > > > Should the community spend TC resources for  prototype?
> > > > > > You always can check tests for your prototype locally.
> > > > > >
> > > > > > And when it's ready, at least from code style point of view
run
> it on TC.
> > > > > >
> > > > > > I, personally, always try to follow project code style, even
for
> > > > > prototypes.
> > > > > > But, If our code style is not convinient for every day coding
> for many
> > > > > > contributors, should you initiate discussion to change it?
> > > > > >
> > > > > >
> > > > > > ср, 13 февр. 2019 г. в 16:45, Павлухин Иван
<vololo100@gmail.com
> >:
> > > > > >
> > > > > > > Maxim,
> > > > > > >
> > > > > > > Oh, my poor tabs.. Joke.
> > > > > > >
> > > > > > > I am totally ok with currently enabled checks. But I am
mostly
> > > > > > > concerned about a general approach. I would like to outline
> one thing.
> > > > > > > Requirements for a prototype code are not the same as for
a
> patch
> > > > > > > ready to merge (see a little bit more in the end of that
> message).
> > > > > > >
> > > > > > > We have a document defining code style which every contributor
> should
> > > > > > > follow [1]. And many points can be checked automatically.
> Personally,
> > > > > > > I do not see much need in writing good javadocs for prototype.
> Why
> > > > > > > should I stub it to be able run any build on TC?
> > > > > > >
> > > > > > > Also, we a have a review process which should be applied
to
> every
> > > > > > > patch. Partially it is described in [2]. And due to this
> process every
> > > > > > > patch should not introduce new failures on TC. So, the
patch
> should
> > > > > > > not be merged if inspections failed.
> > > > > > >
> > > > > > > P.S. Something more about prototypes and production code.
> There is a
> > > > > > > common bad practice in software engineering. It is turning
> prototypes
> > > > > > > into production code. Often it is much faster to create
a
> prototype by
> > > > > > > price of violating some rules of writing "clean code".
And
> often
> > > > > > > prototype after successful piloting is turned into production
> code.
> > > > > > > And it is very easy in practice to keep some pieces of
> initially
> > > > > > > "dirty" prototype code. I believe human factor plays a
great
> role
> > > > > > > here. How should it be done right then? In my opinion good
> production
> > > > > > > code should be designed as "good production code" from
the
> beginning.
> > > > > > > So, only ideas are taken from the prototype and a code
is fully
> > > > > > > rewritten.
> > > > > > >
> > > > > > > [1]
> > > > >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines
> > > > > > > [2]
> > > > >
> https://cwiki.apache.org/confluence/display/IGNITE/Review+Checklist
> > > > > > >
> > > > > > > ср, 13 февр. 2019 г. в 15:05, Maxim Muzafarov <
> maxmuzaf@gmail.com>:
> > > > > > > >
> > > > > > > > Ivan,
> > > > > > > >
> > > > > > > > As the first implementation of this addition, I'd
prefer to
> make it
> > > > > > > > working like _Licenses Headers_ suite check. It will
fail
> when some
> > > > > of
> > > > > > > > the code style checks violated. Moreover, these licenses
> header
> > > > > checks
> > > > > > > > can be included in the checkstyle plugin configuration.
> > > > > > > >
> > > > > > > > In general, I'd prefer to have a compilation fail
error with
> code
> > > > > > > > style checks and after we will get a stable checkstyle
suite
> I
> > > > > propose
> > > > > > > > to change it in a "compilation error" way. If we are
talking
> about
> > > > > the
> > > > > > > > coding style convenient for most of the community
members I
> see no
> > > > > > > > difference with coding sketches or production-ready
branches
> equally.
> > > > > > > > Indeed, no one will be against unused imports [or
spaces
> instead of
> > > > > > > > tabs :-) ] in their PRs or prototypes, right? (for
instance,
> it can
> > > > > be
> > > > > > > > automatically removed by IDE at commit phase).
> > > > > > > >
> > > > > > > > Please, note currently enabled checks are:
> > > > > > > >  - list.isEmpty() instead of list.size() == 0
> > > > > > > >  - unused imports
> > > > > > > >  - missing @Override
> > > > > > > >  - sotred modifiers checks (e.g. pulic static final
..)
> > > > > > > >  - redundunt suppersion checks
> > > > > > > >  - spaces insted of tabs.
> > > > > > > >
> > > > > > > > Are you really what to violate these checks in your
> sketches? Hope
> > > > > not
> > > > > > > :-)
> > > > > > > >
> > > > > > > > On Wed, 13 Feb 2019 at 10:25, Nikolay Izhikov <
> nizhikov@apache.org>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Actually, I dont see anything wrong with failing
> *compilation*
> > > > > task.
> > > > > > > > >
> > > > > > > > > I think one should use project code style for
everyday
> coding, not
> > > > > > > only for
> > > > > > > > > ready-to-merge PRs.
> > > > > > > > >
> > > > > > > > > If we cant use code style for everyday coding,
we should
> change the
> > > > > > > > > codestyle.
> > > > > > > > >
> > > > > > > > > What do you think?
> > > > > > > > >
> > > > > > > > > ср, 13 февр. 2019 г., 10:11 Petr Ivanov
> mr.weider@gmail.com:
> > > > > > > > >
> > > > > > > > > > I guess that was about failing build configuration
with
> > > > > Checkstype,
> > > > > > > not
> > > > > > > > > > compilation build itself.
> > > > > > > > > >
> > > > > > > > > > > On 12 Feb 2019, at 18:03, Павлухин
Иван <
> vololo100@gmail.com>
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Folks,
> > > > > > > > > > >
> > > > > > > > > > > Are you going to fail job compiling
Ignite sources [1]
> if some
> > > > > > > > > > > inspection found a problem? Can we
avoid it? It is
> quite common
> > > > > > > > > > > pattern to start some feature implementation
with
> making a
> > > > > sketch
> > > > > > > and
> > > > > > > > > > > running tests against it. I found it
convenient to
> skip some
> > > > > style
> > > > > > > > > > > requirements for such sketches (e.g.
well formed
> javadocs).
> > > > > > > > > > >
> > > > > > > > > > > [1]
> > > > > > > > > >
> > > > > > >
> > > > >
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_BuildApacheIgnite
> > > > > > > > > > >
> > > > > > > > > > > пн, 11 февр. 2019 г. в 11:38,
Nikolay Izhikov <
> > > > > nizhikov@apache.org
> > > > > > > >:
> > > > > > > > > > >>
> > > > > > > > > > >> Petr, we should have 1 configuration
for project, may
> be 1
> > > > > > > configuration
> > > > > > > > > > >> per programming language.
> > > > > > > > > > >>
> > > > > > > > > > >> пн, 11 февр. 2019 г., 11:33
Petr Ivanov
> mr.weider@gmail.com:
> > > > > > > > > > >>
> > > > > > > > > > >>> I was asking about how many
build configuration is
> intended?
> > > > > One
> > > > > > > for
> > > > > > > > > > all
> > > > > > > > > > >>> and multiple per module?
> > > > > > > > > > >>>
> > > > > > > > > > >>> With IDEA inspections it was
going to be build
> configuration
> > > > > per
> > > > > > > > > > module.
> > > > > > > > > > >>>
> > > > > > > > > > >>>
> > > > > > > > > > >>>
> > > > > > > > > > >>>
> > > > > > > > > > >>>> On 11 Feb 2019, at 11:24,
Nikolay Izhikov <
> > > > > nizhikov@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> Hello, Petr.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> Are you saying that we
have not single build task?
> And each
> > > > > > > module
> > > > > > > > > > builds
> > > > > > > > > > >>>> when it required? If yes,
then I propose to create
> a task
> > > > > like
> > > > > > > > > > "Licence
> > > > > > > > > > >>>> check" which will be run
for every patch.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> My point is that violation
of codestyle should be
> treated as
> > > > > > > hard as
> > > > > > > > > > >>>> compile error.
> > > > > > > > > > >>>>
> > > > > > > > > > >>>> пн, 11 февр. 2019
г., 11:16 Petr Ivanov
> mr.weider@gmail.com
> > > > > :
> > > > > > > > > > >>>>
> > > > > > > > > > >>>>> Is build configuration
Inspections [Core] meant to
> > > > > transform
> > > > > > > into
> > > > > > > > > > single
> > > > > > > > > > >>>>> all-modules check build
configuration (without
> module
> > > > > > > subdivision)?
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>>>> On 11 Feb 2019,
at 11:02, Nikolay Izhikov <
> > > > > > > nizhikov@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> Hello, Maxim.
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> +1 from me for
migrating to checkstyle.
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> Oleg, there is
plugin for IDEA with 2mln
> downloads -
> > > > > > > > > > >>>>>>
> https://plugins.jetbrains.com/plugin/1065-checkstyle-idea
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> I propose do the
following:
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> 1. Migrate current
checks to checkstyle.
> > > > > > > > > > >>>>>> 2. Apply checks
to all Ignite modules. Currently,
> only
> > > > > core
> > > > > > > module
> > > > > > > > > > are
> > > > > > > > > > >>>>>> checked.
> > > > > > > > > > >>>>>> I will review and
commit this patch, or do it by
> my own.
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> 3. Include code
style checks to "Build Apache
> Ignite"
> > > > > suite.
> > > > > > > Ignite
> > > > > > > > > > has
> > > > > > > > > > >>>>> to
> > > > > > > > > > >>>>>> fail to build if
patch violates codestyle.
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>> вс, 10 февр.
2019 г. в 07:54, Павлухин Иван <
> > > > > > > vololo100@gmail.com>:
> > > > > > > > > > >>>>>>
> > > > > > > > > > >>>>>>> Hi,
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>>> I also think
that some warning from IDEA that
> some code
> > > > > > > style rule
> > > > > > > > > > is
> > > > > > > > > > >>>>>>> violated is
a must-have.
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>>> вс, 10 февр.
2019 г. в 01:58, oignatenko <
> > > > > > > oignatenko@gridgain.com
> > > > > > > > > > >:
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> Hi Maxim,
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> I believe
that whatever style checks we
> establish at
> > > > > > > Teamcity, we
> > > > > > > > > > >>>>> better
> > > > > > > > > > >>>>>>>> take care
of making it easy for developers to
> find and
> > > > > fix
> > > > > > > > > > violations
> > > > > > > > > > >>>>> in
> > > > > > > > > > >>>>>>>> their typical
dev environment (for Ignite this
> means, in
> > > > > > > IDEA). I
> > > > > > > > > > >>> think
> > > > > > > > > > >>>>>>> it
> > > > > > > > > > >>>>>>>> is important
that developers can maintain
> required style
> > > > > > > with
> > > > > > > > > > minimal
> > > > > > > > > > >>>>>>> effort
> > > > > > > > > > >>>>>>>> on their
side.
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> If above
is doable then I am 200% for migrating
> our
> > > > > Teamcity
> > > > > > > > > > >>>>> inspections
> > > > > > > > > > >>>>>>> to
> > > > > > > > > > >>>>>>>> checkstyle
/ maven.
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> This is
because I am very disappointed
> observing how it
> > > > > > > stays
> > > > > > > > > > broken
> > > > > > > > > > >>>>> for
> > > > > > > > > > >>>>>>> so
> > > > > > > > > > >>>>>>>> long. And
worst of all, even when (if) it is
> fixed, I
> > > > > feel
> > > > > > > we will
> > > > > > > > > > >>>>>>> always be
> > > > > > > > > > >>>>>>>> at risk
that it breaks again and that we will
> have to
> > > > > again
> > > > > > > wait
> > > > > > > > > > for
> > > > > > > > > > >>>>>>> months
> > > > > > > > > > >>>>>>>> for it
to be fixed.
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> This is
such a stark contrast with my experience
> > > > > regarding
> > > > > > > > > > checkstyle
> > > > > > > > > > >>>>>>> based
> > > > > > > > > > >>>>>>>> inspections.
These just work and you just never
> fear
> > > > > that
> > > > > > > it is
> > > > > > > > > > going
> > > > > > > > > > >>>>> to
> > > > > > > > > > >>>>>>>> break for
some obscure reason, this is so much
> better
> > > > > than
> > > > > > > what I
> > > > > > > > > > >>>>> observe
> > > > > > > > > > >>>>>>>> now.
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> One suggestion
in case if we pick checkstyle - I
> > > > > recommend
> > > > > > > keeping
> > > > > > > > > > >>> its
> > > > > > > > > > >>>>>>>> config
file somewhere in the project under
> version
> > > > > control.
> > > > > > > I
> > > > > > > > > > used to
> > > > > > > > > > >>>>>>>> maintain
such a shared style config at one of
> past jobs
> > > > > and
> > > > > > > after
> > > > > > > > > > >>> some
> > > > > > > > > > >>>>>>>> experimenting
it turned out most convenient to
> have it
> > > > > this
> > > > > > > way -
> > > > > > > > > > so
> > > > > > > > > > >>>>> that
> > > > > > > > > > >>>>>>>> developers
could easily assess and discuss style
> > > > > settings
> > > > > > > and keep
> > > > > > > > > > >>>>> track
> > > > > > > > > > >>>>>>> of
> > > > > > > > > > >>>>>>>> changes
in these. (note how Kafka folks from
> your link
> > > > > [5]
> > > > > > > appear
> > > > > > > > > > to
> > > > > > > > > > >>> be
> > > > > > > > > > >>>>>>>> doing it
this way)
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> regards,
Oleg
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> Mmuzaf
wrote
> > > > > > > > > > >>>>>>>>> Igniters,
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> I've
found that some of the community members
> have
> > > > > faced
> > > > > > > with
> > > > > > > > > > >>>>>>>>> `[Inspections]
Core suite [1]` is not working
> well
> > > > > enough
> > > > > > > on TC.
> > > > > > > > > > The
> > > > > > > > > > >>>>>>>>> suite
has a `FAILED` status for more than 2
> months due
> > > > > to
> > > > > > > some
> > > > > > > > > > >>> issues
> > > > > > > > > > >>>>>>>>> in
TeamCity application [2]. Current suite
> behaviour
> > > > > > > confuses not
> > > > > > > > > > >>> only
> > > > > > > > > > >>>>>>>>> new
contributors but also other community
> members.
> > > > > > > Moreover, this
> > > > > > > > > > >>>>>>>>> suite
is no longer checks rules we previously
> > > > > configured.
> > > > > > > For
> > > > > > > > > > >>>>>>>>> instance,
in the master branch, I've found 11
> `Unused
> > > > > > > imports`
> > > > > > > > > > which
> > > > > > > > > > >>>>>>>>> should
have been caught earlier (e.g. for
> > > > > > > > > > >>>>>>>>> {{IgniteCachePutAllRestartTest}
[3]).
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> I think
we should make the next step to enable
> an
> > > > > > > automatic code
> > > > > > > > > > >>> style
> > > > > > > > > > >>>>>>>>> checks.
As an example, we can consider the
> Apache Kafka
> > > > > > > code
> > > > > > > > > > style
> > > > > > > > > > >>> [5]
> > > > > > > > > > >>>>>>>>> way
and configure for the Ignite project a
> > > > > > > > > > maven-checkstyle-plugin
> > > > > > > > > > >>>>>>>>> with
its own maven profile and run it
> simultaneously
> > > > > with
> > > > > > > other
> > > > > > > > > > TC.
> > > > > > > > > > >>> We
> > > > > > > > > > >>>>>>>>> can
also enable the previously configured
> inspection
> > > > > > > rules, so no
> > > > > > > > > > >>>>>>>>> coding
style violations will be missed.
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> I see
some advantages of using a maven plugin:
> > > > > > > > > > >>>>>>>>> - an
IDE agnostic way for code checks
> > > > > > > > > > >>>>>>>>> - can
be used with different CI and build tools
> > > > > (Jenkins,
> > > > > > > TC)
> > > > > > > > > > >>>>>>>>> - executable
from the command line
> > > > > > > > > > >>>>>>>>> - the
entry single point to configure new rules
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> I've
created the ticket [4] and will prepare
> PR for it.
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> WDYT?
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> [1]
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>
> > > > > > > > > >
> > > > > > >
> > > > >
> https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_InspectionsCore&branch_IgniteTests24Java8=%3Cdefault%3E&tab=buildTypeStatusDiv
> > > > > > > > > > >>>>>>>>> [2]
> https://youtrack.jetbrains.com/issue/TW-58504
> > > > > > > > > > >>>>>>>>> [3]
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>
> > > > > > > > > >
> > > > > > >
> > > > >
> https://github.com/apache/ignite/blob/master/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteCachePutAllRestartTest.java#L29
> > > > > > > > > > >>>>>>>>> [4]
> https://issues.apache.org/jira/browse/IGNITE-11277
> > > > > > > > > > >>>>>>>>> [5]
> > > > > https://github.com/apache/kafka/tree/trunk/checkstyle
> > > > > > > > > > >>>>>>>>>
> > > > > > > > > > >>>>>>>>> On
Fri, 21 Dec 2018 at 16:03, Petr Ivanov &lt;
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> mr.weider@
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> &gt;
wrote:
> > > > > > > > > > >>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>
It seems there is bug in latest 2018.2
> TeamCity
> > > > > > > > > > >>>>>>>>>>
Bug is filed [1]
> > > > > > > > > > >>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>
[1]
> https://youtrack.jetbrains.com/issue/TW-58504
> > > > > > > > > > >>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>
On 19 Dec 2018, at 11:31, Petr Ivanov &lt;
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> mr.weider@
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> &gt;
wrote:
> > > > > > > > > > >>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>
Investigating problem, stand by.
> > > > > > > > > > >>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>>
On 18 Dec 2018, at 19:41, Dmitriy Pavlov
> &lt;
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> dpavlov@
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>> &gt;
wrote:
> > > > > > > > > > >>>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>>
Both patches were applied. Maxim, thank you!
> > > > > > > > > > >>>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>>
What about 1. An `Unexpected error during
> build
> > > > > messages
> > > > > > > > > > >>>>>>> processing
in
> > > > > > > > > > >>>>>>>>>>>>
TeamCity`, what can we do as the next step
> to fix
> > > > > it?
> > > > > > > > > > >>>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>>>
Sincerely,
> > > > > > > > > > >>>>>>>>>>>>
Dmitriy Pavlov
> > > > > > > > > > >>>>>>>>>>>>
[cut]
> > > > > > > > > > >>>>>>>>>>>
> > > > > > > > > > >>>>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>>
> > > > > > > > > > >>>>>>>> --
> > > > > > > > > > >>>>>>>> Sent from:
> > > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>>> --
> > > > > > > > > > >>>>>>> Best regards,
> > > > > > > > > > >>>>>>> Ivan Pavlukhin
> > > > > > > > > > >>>>>>>
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>>>
> > > > > > > > > > >>>
> > > > > > > > > > >>>
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Best regards,
> > > > > > > > > > > Ivan Pavlukhin
> > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best regards,
> > > > > > > Ivan Pavlukhin
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Ivan Pavlukhin
> > > > >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>
-- 
--
Maxim Muzafarov

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