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 > >>>> > >> > >> > >
