Hi Maxim, > Why the auto-formatting procedure cannot be used for any PR you'd like to run on TC? > Even more, you can remove all the rules from checkstyle XML config and do all you want in the PR branch Yes, I can enable auto-formatting, which should be checked anyway, I can edit pom.xml every time, etc.
My question is the following: Why can't this check be done in parallel with other tasks? Yep, I see the argument mentioned by Nikolay - TeamCity capacity (we should re-run all tests even though we rename a variable), but I don't see that our TC servers are overwhelmed for now. Perhaps, I am wrong and this argument (TC capacity) is more significant than it seems to me at first glance. Folks, please don't get me wrong. I am not against the code-style check at all. I just want to get some freedom for dev branches only, if it is possible of course. Thanks, Slava. вт, 7 июл. 2020 г. в 12:21, Maxim Muzafarov <mmu...@apache.org>: > Slava, > > Why the auto-formatting procedure cannot be used for any PR you'd like > to run on TC? Even more, you can remove all the rules from checkstyle > XML config and do all you want in the PR branch. > > On Tue, 7 Jul 2020 at 12:17, Вячеслав Коптилин <slava.kopti...@gmail.com> > wrote: > > > > 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 <nizhi...@apache.org>: > > > > > > 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, Вячеслав Коптилин <slava.kopti...@gmail.com > > > > > написал(а): > > > > > > > > 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 <nizhi...@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.kopti...@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 <nizhi...@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.kopti...@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 <mmu...@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.kasnach...@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 > > > >>>>>> > > > >>>> > > > >>>> > > > >> > > > >> > > > > > > >
