Hi Nikolay, > Why do you need to Run All on TC resources for this task? For instance, it may be a race that cannot be reproduced locally on my own hardware. (By the way, even though if I just want to start Cache1 suite, the Code-Style check executes anyway).
> If we don’t want to follow the code style or considered it as a «waste of the time» we should change it. This is absolutely not what I'm trying to say. I do not think, that code style is useless. I just want to say that this check can be done in parallel for dev branches, for example. Thanks, S. вт, 7 июл. 2020 г. в 11:49, Nikolay Izhikov <[email protected]>: > > Let's assume that I want to implement a dirty fix of a bug, check a > reproducer from the user list, etc. > > Why do you need to Run All on TC resources for this task? > > > I do not want to waste my time fixing all code style violations. > > I assume that you have the Ignite project locally because you edit Ignite > source code. > If yes, then IntelliJ IDEA has an automatic code formatting and does all > the work for you. > > > Does this use-case look reasonable? > > Yes. > But, I still don’t see what is the issue here. > > If we don’t want to follow the code style or considered it as a «waste of > the time» we should change it. > As simple as that. > > But, we should force the code style as early as we can. > I think we should fail maven local build on code style violations. > > > 7 июля 2020 г., в 11:28, Вячеслав Коптилин <[email protected]> > написал(а): > > > > 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 <[email protected]>: > > > >> 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, Вячеслав Коптилин <[email protected]> > >> написал(а): > >>> > >>> 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 <[email protected]>: > >>> > >>>> 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, Вячеслав Коптилин <[email protected] > > > >>>> написал(а): > >>>>> > >>>>> 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 <[email protected]>: > >>>>> > >>>>>> 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 < > >> [email protected] > >>>>> > >>>>>> 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 > >>>>>> > >>>> > >>>> > >> > >> > >
