ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Вячеслав Коптилин <slava.kopti...@gmail.com>
Subject Re: Checkstyle fails Build Apache Ignite - can we split it?
Date Tue, 07 Jul 2020 08:28:12 GMT
Nikolay,

There is *NO *intention to ignore code style violations and do merge PRs
into the master branch without fixing them.

Let's assume that I want to implement a dirty fix of a bug, check a
reproducer from the user list, etc.
And I do not have the intention to merge that into the master branch as is,
however, I do not want to waste my time fixing all code style violations.
Does this use-case look reasonable?

Thanks,
Slava.

вт, 7 июл. 2020 г. в 11:07, Nikolay Izhikov <nizhikov@apache.org>:

> Slava.
>
> All contributors should follow our code style during their contribution.
> For now, we have an automatic PR check that is integrated to the GitHub
> interface.
>
> I don’t see any issue here.
> All open-source project that I know, uses the same approach.
>
> Anyway, If some of the experienced community members is interested in the
> particular contribution he or she can help a newcomer with the code style -
> GitHub interface has the edit button even for someone else’s PR’s
>
>
> > 7 июля 2020 г., в 11:01, Вячеслав Коптилин <slava.koptilin@gmail.com>
> написал(а):
> >
> > Hello Nikolay,
> >
> >> Because any code style violations should be fixed before the merge.
> >> Therefore after the fix, you must rerun TC.
> >> This means that running heavy suites when code style is violated is a
> > waste of the resources.
> > This makes sense, however, to be honest, I don't see that our team city
> > servers are really busy.
> >
> >> Why do you want to violate code style rules in your PR?
> > Please take a look at the original e-mail from Ilya:
> >> This means that I have completely lost an option to run tests against
> > pull
> >> requests by new contributors - they usually compile but will not pass
> >> Checkstyle. That's a blocker.
> >
> > This issue has also been discussed here:
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/DISCUSSION-Separate-code-sanity-check-and-build-task-td47003.html
> >
> > Thanks,
> > Slava.
> >
> >
> > вт, 7 июл. 2020 г. в 10:47, Nikolay Izhikov <nizhikov@apache.org>:
> >
> >> All checks just force rules we agreed on.
> >>
> >>> Why this check is so important? Why do you think it is more important
> >> than all other tasks/tests?
> >>
> >> Because any code style violations should be fixed before the merge.
> >> Therefore after the fix, you must rerun TC.
> >> This means that running heavy suites when code style is violated is a
> >> waste of the resources.
> >>
> >> Why do you want to violate code style rules in your PR?
> >>
> >> I think instead of hiding style errors we should make our code style
> >> comfortable for everyone.
> >> Can you, please, write - what part of the code style hurts you?
> >>
> >>
> >>> 7 июля 2020 г., в 10:39, Вячеслав Коптилин <slava.koptilin@gmail.com>
> >> написал(а):
> >>>
> >>> Hello Maxim,
> >>>
> >>>> Why do you think that splitting the checkstyle build is better option
> >>> than fixing code style issues reporting by the checkstyle plugin?
> >>> Why do you think that Ilya talks that code style errors should not be
> >> fixed?
> >>>
> >>> It looks weird to me that we do not even start the tests if check style
> >>> plugin reports violations.
> >>> Why can't this check be done in parallel with the tests and reported by
> >>> mtcga bot?
> >>> Why this check is so important? Why do you think it is more important
> >> than
> >>> all other tasks/tests?
> >>>
> >>> Thanks,
> >>> Slava.
> >>>
> >>> пн, 6 июл. 2020 г. в 20:34, Maxim Muzafarov <mmuzaf@apache.org>:
> >>>
> >>>> Hello Ilya,
> >>>>
> >>>> Why do you think that splitting the checkstyle build is better option
> >>>> than fixing code style issues reporting by the checkstyle plugin?
> >>>>
> >>>> On Mon, 6 Jul 2020 at 19:43, Ilya Kasnacheev <
> ilya.kasnacheev@gmail.com
> >>>
> >>>> wrote:
> >>>>>
> >>>>> Hello!
> >>>>>
> >>>>> I have just noticed today that Checkstyle will fail Apache Ignite
> >> build:
> >>>>>
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite/5443282?buildTab=log&focusLine=3&linesState=683.4289
> >>>>>
> >>>>> This means that I have completely lost an option to run tests against
> >>>> pull
> >>>>> requests by new contributors - they usually compile but will not
pass
> >>>>> Checkstyle. That's a blocker.
> >>>>>
> >>>>> Can we please split Checkstyle as a separate build which is triggered
> >>>> with
> >>>>> Run All?
> >>>>> I think we even have
> >>>>>
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle?mode=builds#all-projects
> >>>>>
> >>>>> WDYT?
> >>>>>
> >>>>> Regards,
> >>>>> --
> >>>>> Ilya Kasnacheev
> >>>>
> >>
> >>
>
>

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