> > I am not against the code-style check at all. > Yes, you are :) But it’s ok No, I am not :)
> You can take a look at the commits that introduced checks and how many files were touched. I really appreciate this, Nikolay. Thanks, S. вт, 7 июл. 2020 г. в 13:04, Nikolay Izhikov <nizhi...@apache.org>: > > Why can't this check be done in parallel with other tasks? > > Because when we contribute code to Ignite we must follow code style. > > > I am not against the code-style check at all. > > Yes, you are :) But it’s ok > > > I just want to get some freedom for dev branches only, if it is possible > of course. > > It’s not about freedom - you are free to run any checks you want on TC. > It’s about project codebase consistency. > > No long time ago we don’t have TC bot or checkstyle plugin. > That leads us to new and new tests failures and code style violations. > > You can take a look at the commits that introduced checks and how many > files were touched. > We just ignore our own rules without checks. > > > https://github.com/apache/ignite/commit/2fbbb676386515ea881e4e61f08864d6bc93225a > > https://github.com/apache/ignite/commit/2a85925f1705fbad36b5421c0ca5cf9de9a29658 > > https://github.com/apache/ignite/commit/d63f4d3569dcb387394d367a7f00aaf35f1b288e > > > > > 7 июля 2020 г., в 12:52, Вячеслав Коптилин <slava.kopti...@gmail.com> > написал(а): > > > > 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 > >>>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >> > >
