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, Вячеслав Коптилин <[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 >> >>>> >> >> >> >> >> >> > -- Best regards, Ivan Pavlukhin
