Ilya, Perhaps I expressed my thoughts vaguely. I have nothing against discussion. I suggest to pin and describe a decision if/when it is made.
2020-07-07 17:10 GMT+03:00, Ilya Kasnacheev <ilya.kasnach...@gmail.com>: > Hello! > > Ivan, unfortunately, I can't see any formal decision being taken. All I see > in the referenced thread is people who are unhappy with obligatory code > style checking as a prerequisite to running tests. > > Did we hold a formal vote on this issue? Did we even hold an informal vote? > It may turn up that it was a voluntary decision by a sole fellow > committer which deserves not pinning but discussion. > > Regards, > -- > Ilya Kasnacheev > > > вт, 7 июл. 2020 г. в 11:50, Ivan Pavlukhin <vololo...@gmail.com>: > >> Folks, >> >> This discussion reoccurs from month to month. Last one I remember is >> [1]. It sounds useful to pin a decision to our coding guidelines. What >> do you think? >> >> [1] >> http://apache-ignite-developers.2346864.n4.nabble.com/DISCUSSION-Separate-code-sanity-check-and-build-task-td47003.html >> >> 2020-07-07 11:28 GMT+03:00, Вячеслав Коптилин <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 >> >> >>>> >> >> >> >> >> >> >> >> >> >> >> > >> >> >> -- >> >> Best regards, >> Ivan Pavlukhin >> > -- Best regards, Ivan Pavlukhin
