You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Konstantin Orlov <ko...@gridgain.com> on 2020/04/20 08:00:20 UTC

[DISCUSSION] Separate code sanity check and build task

Igniters,

Currently we have code sanity checks [1][2] integrated within a build task [3]. Do we really need to fail the build (and therefore the other tasks) if there is a minor flaw like a missing line at the end of a file or an unused import? As for me it could be separated from the build task.

What do you think?

[1] https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle>
[2] https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders>
[3] https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite>

-- 
Regards,
Konstantin Orlov



Re: [DISCUSSION] Separate code sanity check and build task

Posted by Maxim Muzafarov <mm...@apache.org>.
Andrey,

I see the following options for PR checks (without intention to merge it):
1. Disable the checkstyle plugin in pom.xml or remove all rules from
checkstyle.xml (for PR branch).
2.
- import checkstyle.xml cofiguration to IntilliJ IDEA inspections
(Preferences > Editor > Inspections)
- be sure that checkbox is switched on for "Checkstyle real-time scan"
- press Ctrl + Alt + L prior to pushing changes to remote (you can
also enable this by default prior to each push)
- PROFIT?!

In general, I don't see any problems at all. What can be simpler of
pressing Crtl + Alt + L prior to the push?

P.S. Sorry for sending an uncomplete e-mail.

On Mon, 8 Jun 2020 at 18:23, Maxim Muzafarov <mm...@apache.org> wrote:
>
> Andrey,
>
> I see the following options for PR checks (without intention to merge it):
> 1.
>
> On Mon, 8 Jun 2020 at 17:59, Andrey Mashenkov
> <an...@gmail.com> wrote:
> >
> > HI,
> >
> > > Why do you want to run a reproducer from the user list on the TC?
> > It may be useful to verify a race that can't be reproduced locally on my NB
> > due to some performance reasons.
> >
> > > If all code style rules are good then we should use them in daily coding,
> > isn’t it?
> > Yes, also we do not force users to rewrite their code regarding our own
> > code style.
> > But I found the code-style (checkstyle, license and inspections) rules
> > force us to do unnecessary work in this particular case,
> > thus makes contribution a bit harder.
> >
> >
> > On Mon, Jun 8, 2020 at 5:27 PM Nikolay Izhikov <ni...@apache.org> wrote:
> >
> > > Hello, Andrey.
> > >
> > > Sorry, I still don’t understand your use-case.
> > > Can you, please, use correct quoting so I can parse your message? :)
> > >
> > > Why do you want to run a reproducer from the user list on the TC?
> > >
> > > > There is no specific rule.
> > >
> > > If all code style rules are good then we should use them in daily coding,
> > > isn’t it?
> > >
> > > Anyway, I think forcing code style rules as early as we can is a good
> > > thing.
> > > Some open-source projects(Kafka, for example) forces code style rules even
> > > on a local test run.
> > >
> > > > 8 июня 2020 г., в 17:22, Andrey Mashenkov <an...@gmail.com>
> > > написал(а):
> > > >
> > > > Hi Niolay,
> > > >
> > > > Why do you ignore code style rules while developing «quick reproducer»?
> > > >
> > > > I don't think it worth effor to fix a user code (e.g. from userlist) just
> > > > to validate some race condition.
> > > >
> > > > There is no specific rule.
> > > > I'm sad we have no ability to run user code without using any hacks to
> > > skip
> > > > style checks with no intention to merge such code to master.
> > > >
> > > >
> > > > On Mon, Jun 8, 2020 at 4:50 PM Nikolay Izhikov <ni...@apache.org>
> > > wrote:
> > > >
> > > >> Hello, Andrey.
> > > >>
> > > >>> I've just found that I should waste my time fixing styles in user code
> > > >> that (may be) reproduce some bug, just to validate the bug or fix
> > > provided
> > > >> by the user.
> > > >>
> > > >> Why do you ignore code style rules while developing «quick reproducer»?
> > > >>
> > > >> What specific code style rule is an issue for you?
> > > >> If we have some rules is just a waste of the time - may be it better to
> > > >> remove them?
> > > >>
> > > >> I’m ++1 to fail the build on code style errors.
> > > >> Code style errors == compile errors for me.
> > > >>
> > > >>> 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com>
> > > >> написал(а):
> > > >>>
> > > >>> Konstantin,
> > > >>>
> > > >>> +1
> > > >>>
> > > >>> I've just found that I should waste my time fixing styles in user code
> > > >> that
> > > >>> (may be) reproduce some bug, just to validate the bug or fix provided
> > > by
> > > >>> the user.
> > > >>> I'm ok with the idea to block commits with style errors to master
> > > branch,
> > > >>> but not for other branches\PR.
> > > >>>
> > > >>> Can anybody explain why commiters should waste their time for this?
> > > >>> Why we even have such a rule to fail build on TC if there is some kind
> > > of
> > > >>> style error?
> > > >>> It looks (like a bullshit) counterintuitive as it is still possible to
> > > >>> commit to master with having style error, but impossible to just build
> > > a
> > > >>> project or to run a test.
> > > >>>
> > > >>>
> > > >>> On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <korlov@gridgain.com
> > > >
> > > >>> wrote:
> > > >>>
> > > >>>> Hi Ivan,
> > > >>>>
> > > >>>> Thanks for your reply! It’s better to get an answer late than never :)
> > > >>>>
> > > >>>> --
> > > >>>> Regards,
> > > >>>> Konstantin Orlov
> > > >>>>
> > > >>>>
> > > >>>>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com>
> > > wrote:
> > > >>>>>
> > > >>>>> Hi Konstantin,
> > > >>>>>
> > > >>>>> Surprisingly, I found your message in a Spam folder (gmail).
> > > >>>>>
> > > >>>>> We had discussions about the subject before. The most recent one and
> > > >>>>> reflecting a current state is [1]. You can find many thoughts and
> > > >>>>> arguments in another discussion [2] (it might be better to start
> > > >>>>> reading from a bottom).
> > > >>>>>
> > > >>>>> [1]
> > > >>>>
> > > >>
> > > https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> > > >>>>> [2]
> > > >>>>
> > > >>
> > > http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> > > >>>>>
> > > >>>>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> > > >>>>>> Igniters,
> > > >>>>>>
> > > >>>>>> Currently we have code sanity checks [1][2] integrated within a
> > > build
> > > >>>> task
> > > >>>>>> [3]. Do we really need to fail the build (and therefore the other
> > > >>>> tasks) if
> > > >>>>>> there is a minor flaw like a missing line at the end of a file or an
> > > >>>> unused
> > > >>>>>> import? As for me it could be separated from the build task.
> > > >>>>>>
> > > >>>>>> What do you think?
> > > >>>>>>
> > > >>>>>> [1]
> > > >>>>>>
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> > > >>>>>> <
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> > > >>>>>
> > > >>>>>> [2]
> > > >>>>>>
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> > > >>>>>> <
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> > > >>>>>
> > > >>>>>> [3]
> > > >>>>>>
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> > > >>>>>> <
> > > >>>>
> > > >>
> > > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> > > >>>>>
> > > >>>>>>
> > > >>>>>> --
> > > >>>>>> Regards,
> > > >>>>>> Konstantin Orlov
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> --
> > > >>>>>
> > > >>>>> Best regards,
> > > >>>>> Ivan Pavlukhin
> > > >>>>
> > > >>>>
> > > >>>
> > > >>> --
> > > >>> Best regards,
> > > >>> Andrey V. Mashenkov
> > > >>
> > > >>
> > > >
> > > > --
> > > > Best regards,
> > > > Andrey V. Mashenkov
> > >
> > >
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov

Re: [DISCUSSION] Separate code sanity check and build task

Posted by Maxim Muzafarov <mm...@apache.org>.
Andrey,

I see the following options for PR checks (without intention to merge it):
1.

On Mon, 8 Jun 2020 at 17:59, Andrey Mashenkov
<an...@gmail.com> wrote:
>
> HI,
>
> > Why do you want to run a reproducer from the user list on the TC?
> It may be useful to verify a race that can't be reproduced locally on my NB
> due to some performance reasons.
>
> > If all code style rules are good then we should use them in daily coding,
> isn’t it?
> Yes, also we do not force users to rewrite their code regarding our own
> code style.
> But I found the code-style (checkstyle, license and inspections) rules
> force us to do unnecessary work in this particular case,
> thus makes contribution a bit harder.
>
>
> On Mon, Jun 8, 2020 at 5:27 PM Nikolay Izhikov <ni...@apache.org> wrote:
>
> > Hello, Andrey.
> >
> > Sorry, I still don’t understand your use-case.
> > Can you, please, use correct quoting so I can parse your message? :)
> >
> > Why do you want to run a reproducer from the user list on the TC?
> >
> > > There is no specific rule.
> >
> > If all code style rules are good then we should use them in daily coding,
> > isn’t it?
> >
> > Anyway, I think forcing code style rules as early as we can is a good
> > thing.
> > Some open-source projects(Kafka, for example) forces code style rules even
> > on a local test run.
> >
> > > 8 июня 2020 г., в 17:22, Andrey Mashenkov <an...@gmail.com>
> > написал(а):
> > >
> > > Hi Niolay,
> > >
> > > Why do you ignore code style rules while developing «quick reproducer»?
> > >
> > > I don't think it worth effor to fix a user code (e.g. from userlist) just
> > > to validate some race condition.
> > >
> > > There is no specific rule.
> > > I'm sad we have no ability to run user code without using any hacks to
> > skip
> > > style checks with no intention to merge such code to master.
> > >
> > >
> > > On Mon, Jun 8, 2020 at 4:50 PM Nikolay Izhikov <ni...@apache.org>
> > wrote:
> > >
> > >> Hello, Andrey.
> > >>
> > >>> I've just found that I should waste my time fixing styles in user code
> > >> that (may be) reproduce some bug, just to validate the bug or fix
> > provided
> > >> by the user.
> > >>
> > >> Why do you ignore code style rules while developing «quick reproducer»?
> > >>
> > >> What specific code style rule is an issue for you?
> > >> If we have some rules is just a waste of the time - may be it better to
> > >> remove them?
> > >>
> > >> I’m ++1 to fail the build on code style errors.
> > >> Code style errors == compile errors for me.
> > >>
> > >>> 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com>
> > >> написал(а):
> > >>>
> > >>> Konstantin,
> > >>>
> > >>> +1
> > >>>
> > >>> I've just found that I should waste my time fixing styles in user code
> > >> that
> > >>> (may be) reproduce some bug, just to validate the bug or fix provided
> > by
> > >>> the user.
> > >>> I'm ok with the idea to block commits with style errors to master
> > branch,
> > >>> but not for other branches\PR.
> > >>>
> > >>> Can anybody explain why commiters should waste their time for this?
> > >>> Why we even have such a rule to fail build on TC if there is some kind
> > of
> > >>> style error?
> > >>> It looks (like a bullshit) counterintuitive as it is still possible to
> > >>> commit to master with having style error, but impossible to just build
> > a
> > >>> project or to run a test.
> > >>>
> > >>>
> > >>> On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <korlov@gridgain.com
> > >
> > >>> wrote:
> > >>>
> > >>>> Hi Ivan,
> > >>>>
> > >>>> Thanks for your reply! It’s better to get an answer late than never :)
> > >>>>
> > >>>> --
> > >>>> Regards,
> > >>>> Konstantin Orlov
> > >>>>
> > >>>>
> > >>>>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com>
> > wrote:
> > >>>>>
> > >>>>> Hi Konstantin,
> > >>>>>
> > >>>>> Surprisingly, I found your message in a Spam folder (gmail).
> > >>>>>
> > >>>>> We had discussions about the subject before. The most recent one and
> > >>>>> reflecting a current state is [1]. You can find many thoughts and
> > >>>>> arguments in another discussion [2] (it might be better to start
> > >>>>> reading from a bottom).
> > >>>>>
> > >>>>> [1]
> > >>>>
> > >>
> > https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> > >>>>> [2]
> > >>>>
> > >>
> > http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> > >>>>>
> > >>>>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> > >>>>>> Igniters,
> > >>>>>>
> > >>>>>> Currently we have code sanity checks [1][2] integrated within a
> > build
> > >>>> task
> > >>>>>> [3]. Do we really need to fail the build (and therefore the other
> > >>>> tasks) if
> > >>>>>> there is a minor flaw like a missing line at the end of a file or an
> > >>>> unused
> > >>>>>> import? As for me it could be separated from the build task.
> > >>>>>>
> > >>>>>> What do you think?
> > >>>>>>
> > >>>>>> [1]
> > >>>>>>
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> > >>>>>> <
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> > >>>>>
> > >>>>>> [2]
> > >>>>>>
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> > >>>>>> <
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> > >>>>>
> > >>>>>> [3]
> > >>>>>>
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> > >>>>>> <
> > >>>>
> > >>
> > https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> > >>>>>
> > >>>>>>
> > >>>>>> --
> > >>>>>> Regards,
> > >>>>>> Konstantin Orlov
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>> --
> > >>>>>
> > >>>>> Best regards,
> > >>>>> Ivan Pavlukhin
> > >>>>
> > >>>>
> > >>>
> > >>> --
> > >>> Best regards,
> > >>> Andrey V. Mashenkov
> > >>
> > >>
> > >
> > > --
> > > Best regards,
> > > Andrey V. Mashenkov
> >
> >
>
> --
> Best regards,
> Andrey V. Mashenkov

Re: [DISCUSSION] Separate code sanity check and build task

Posted by Andrey Mashenkov <an...@gmail.com>.
HI,

> Why do you want to run a reproducer from the user list on the TC?
It may be useful to verify a race that can't be reproduced locally on my NB
due to some performance reasons.

> If all code style rules are good then we should use them in daily coding,
isn’t it?
Yes, also we do not force users to rewrite their code regarding our own
code style.
But I found the code-style (checkstyle, license and inspections) rules
force us to do unnecessary work in this particular case,
thus makes contribution a bit harder.


On Mon, Jun 8, 2020 at 5:27 PM Nikolay Izhikov <ni...@apache.org> wrote:

> Hello, Andrey.
>
> Sorry, I still don’t understand your use-case.
> Can you, please, use correct quoting so I can parse your message? :)
>
> Why do you want to run a reproducer from the user list on the TC?
>
> > There is no specific rule.
>
> If all code style rules are good then we should use them in daily coding,
> isn’t it?
>
> Anyway, I think forcing code style rules as early as we can is a good
> thing.
> Some open-source projects(Kafka, for example) forces code style rules even
> on a local test run.
>
> > 8 июня 2020 г., в 17:22, Andrey Mashenkov <an...@gmail.com>
> написал(а):
> >
> > Hi Niolay,
> >
> > Why do you ignore code style rules while developing «quick reproducer»?
> >
> > I don't think it worth effor to fix a user code (e.g. from userlist) just
> > to validate some race condition.
> >
> > There is no specific rule.
> > I'm sad we have no ability to run user code without using any hacks to
> skip
> > style checks with no intention to merge such code to master.
> >
> >
> > On Mon, Jun 8, 2020 at 4:50 PM Nikolay Izhikov <ni...@apache.org>
> wrote:
> >
> >> Hello, Andrey.
> >>
> >>> I've just found that I should waste my time fixing styles in user code
> >> that (may be) reproduce some bug, just to validate the bug or fix
> provided
> >> by the user.
> >>
> >> Why do you ignore code style rules while developing «quick reproducer»?
> >>
> >> What specific code style rule is an issue for you?
> >> If we have some rules is just a waste of the time - may be it better to
> >> remove them?
> >>
> >> I’m ++1 to fail the build on code style errors.
> >> Code style errors == compile errors for me.
> >>
> >>> 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com>
> >> написал(а):
> >>>
> >>> Konstantin,
> >>>
> >>> +1
> >>>
> >>> I've just found that I should waste my time fixing styles in user code
> >> that
> >>> (may be) reproduce some bug, just to validate the bug or fix provided
> by
> >>> the user.
> >>> I'm ok with the idea to block commits with style errors to master
> branch,
> >>> but not for other branches\PR.
> >>>
> >>> Can anybody explain why commiters should waste their time for this?
> >>> Why we even have such a rule to fail build on TC if there is some kind
> of
> >>> style error?
> >>> It looks (like a bullshit) counterintuitive as it is still possible to
> >>> commit to master with having style error, but impossible to just build
> a
> >>> project or to run a test.
> >>>
> >>>
> >>> On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <korlov@gridgain.com
> >
> >>> wrote:
> >>>
> >>>> Hi Ivan,
> >>>>
> >>>> Thanks for your reply! It’s better to get an answer late than never :)
> >>>>
> >>>> --
> >>>> Regards,
> >>>> Konstantin Orlov
> >>>>
> >>>>
> >>>>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com>
> wrote:
> >>>>>
> >>>>> Hi Konstantin,
> >>>>>
> >>>>> Surprisingly, I found your message in a Spam folder (gmail).
> >>>>>
> >>>>> We had discussions about the subject before. The most recent one and
> >>>>> reflecting a current state is [1]. You can find many thoughts and
> >>>>> arguments in another discussion [2] (it might be better to start
> >>>>> reading from a bottom).
> >>>>>
> >>>>> [1]
> >>>>
> >>
> https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> >>>>> [2]
> >>>>
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> >>>>>
> >>>>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> >>>>>> Igniters,
> >>>>>>
> >>>>>> Currently we have code sanity checks [1][2] integrated within a
> build
> >>>> task
> >>>>>> [3]. Do we really need to fail the build (and therefore the other
> >>>> tasks) if
> >>>>>> there is a minor flaw like a missing line at the end of a file or an
> >>>> unused
> >>>>>> import? As for me it could be separated from the build task.
> >>>>>>
> >>>>>> What do you think?
> >>>>>>
> >>>>>> [1]
> >>>>>>
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >>>>>> <
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >>>>>
> >>>>>> [2]
> >>>>>>
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >>>>>> <
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >>>>>
> >>>>>> [3]
> >>>>>>
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >>>>>> <
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Regards,
> >>>>>> Konstantin Orlov
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>>
> >>>>> Best regards,
> >>>>> Ivan Pavlukhin
> >>>>
> >>>>
> >>>
> >>> --
> >>> Best regards,
> >>> Andrey V. Mashenkov
> >>
> >>
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
>
>

-- 
Best regards,
Andrey V. Mashenkov

Re: [DISCUSSION] Separate code sanity check and build task

Posted by Nikolay Izhikov <ni...@apache.org>.
Hello, Andrey.

Sorry, I still don’t understand your use-case.
Can you, please, use correct quoting so I can parse your message? :)

Why do you want to run a reproducer from the user list on the TC?

> There is no specific rule.

If all code style rules are good then we should use them in daily coding, isn’t it?

Anyway, I think forcing code style rules as early as we can is a good thing.
Some open-source projects(Kafka, for example) forces code style rules even on a local test run.

> 8 июня 2020 г., в 17:22, Andrey Mashenkov <an...@gmail.com> написал(а):
> 
> Hi Niolay,
> 
> Why do you ignore code style rules while developing «quick reproducer»?
> 
> I don't think it worth effor to fix a user code (e.g. from userlist) just
> to validate some race condition.
> 
> There is no specific rule.
> I'm sad we have no ability to run user code without using any hacks to skip
> style checks with no intention to merge such code to master.
> 
> 
> On Mon, Jun 8, 2020 at 4:50 PM Nikolay Izhikov <ni...@apache.org> wrote:
> 
>> Hello, Andrey.
>> 
>>> I've just found that I should waste my time fixing styles in user code
>> that (may be) reproduce some bug, just to validate the bug or fix provided
>> by the user.
>> 
>> Why do you ignore code style rules while developing «quick reproducer»?
>> 
>> What specific code style rule is an issue for you?
>> If we have some rules is just a waste of the time - may be it better to
>> remove them?
>> 
>> I’m ++1 to fail the build on code style errors.
>> Code style errors == compile errors for me.
>> 
>>> 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com>
>> написал(а):
>>> 
>>> Konstantin,
>>> 
>>> +1
>>> 
>>> I've just found that I should waste my time fixing styles in user code
>> that
>>> (may be) reproduce some bug, just to validate the bug or fix provided by
>>> the user.
>>> I'm ok with the idea to block commits with style errors to master branch,
>>> but not for other branches\PR.
>>> 
>>> Can anybody explain why commiters should waste their time for this?
>>> Why we even have such a rule to fail build on TC if there is some kind of
>>> style error?
>>> It looks (like a bullshit) counterintuitive as it is still possible to
>>> commit to master with having style error, but impossible to just build a
>>> project or to run a test.
>>> 
>>> 
>>> On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <ko...@gridgain.com>
>>> wrote:
>>> 
>>>> Hi Ivan,
>>>> 
>>>> Thanks for your reply! It’s better to get an answer late than never :)
>>>> 
>>>> --
>>>> Regards,
>>>> Konstantin Orlov
>>>> 
>>>> 
>>>>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com> wrote:
>>>>> 
>>>>> Hi Konstantin,
>>>>> 
>>>>> Surprisingly, I found your message in a Spam folder (gmail).
>>>>> 
>>>>> We had discussions about the subject before. The most recent one and
>>>>> reflecting a current state is [1]. You can find many thoughts and
>>>>> arguments in another discussion [2] (it might be better to start
>>>>> reading from a bottom).
>>>>> 
>>>>> [1]
>>>> 
>> https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
>>>>> [2]
>>>> 
>> http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
>>>>> 
>>>>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
>>>>>> Igniters,
>>>>>> 
>>>>>> Currently we have code sanity checks [1][2] integrated within a build
>>>> task
>>>>>> [3]. Do we really need to fail the build (and therefore the other
>>>> tasks) if
>>>>>> there is a minor flaw like a missing line at the end of a file or an
>>>> unused
>>>>>> import? As for me it could be separated from the build task.
>>>>>> 
>>>>>> What do you think?
>>>>>> 
>>>>>> [1]
>>>>>> 
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
>>>>>> <
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
>>>>> 
>>>>>> [2]
>>>>>> 
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
>>>>>> <
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
>>>>> 
>>>>>> [3]
>>>>>> 
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
>>>>>> <
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
>>>>> 
>>>>>> 
>>>>>> --
>>>>>> Regards,
>>>>>> Konstantin Orlov
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> 
>>>>> Best regards,
>>>>> Ivan Pavlukhin
>>>> 
>>>> 
>>> 
>>> --
>>> Best regards,
>>> Andrey V. Mashenkov
>> 
>> 
> 
> -- 
> Best regards,
> Andrey V. Mashenkov


Re: [DISCUSSION] Separate code sanity check and build task

Posted by Andrey Mashenkov <an...@gmail.com>.
Hi Niolay,

Why do you ignore code style rules while developing «quick reproducer»?

I don't think it worth effor to fix a user code (e.g. from userlist) just
to validate some race condition.

There is no specific rule.
I'm sad we have no ability to run user code without using any hacks to skip
style checks with no intention to merge such code to master.


On Mon, Jun 8, 2020 at 4:50 PM Nikolay Izhikov <ni...@apache.org> wrote:

> Hello, Andrey.
>
> > I've just found that I should waste my time fixing styles in user code
> that (may be) reproduce some bug, just to validate the bug or fix provided
> by the user.
>
> Why do you ignore code style rules while developing «quick reproducer»?
>
> What specific code style rule is an issue for you?
> If we have some rules is just a waste of the time - may be it better to
> remove them?
>
> I’m ++1 to fail the build on code style errors.
> Code style errors == compile errors for me.
>
> > 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com>
> написал(а):
> >
> > Konstantin,
> >
> > +1
> >
> > I've just found that I should waste my time fixing styles in user code
> that
> > (may be) reproduce some bug, just to validate the bug or fix provided by
> > the user.
> > I'm ok with the idea to block commits with style errors to master branch,
> > but not for other branches\PR.
> >
> > Can anybody explain why commiters should waste their time for this?
> > Why we even have such a rule to fail build on TC if there is some kind of
> > style error?
> > It looks (like a bullshit) counterintuitive as it is still possible to
> > commit to master with having style error, but impossible to just build a
> > project or to run a test.
> >
> >
> > On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <ko...@gridgain.com>
> > wrote:
> >
> >> Hi Ivan,
> >>
> >> Thanks for your reply! It’s better to get an answer late than never :)
> >>
> >> --
> >> Regards,
> >> Konstantin Orlov
> >>
> >>
> >>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com> wrote:
> >>>
> >>> Hi Konstantin,
> >>>
> >>> Surprisingly, I found your message in a Spam folder (gmail).
> >>>
> >>> We had discussions about the subject before. The most recent one and
> >>> reflecting a current state is [1]. You can find many thoughts and
> >>> arguments in another discussion [2] (it might be better to start
> >>> reading from a bottom).
> >>>
> >>> [1]
> >>
> https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> >>> [2]
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> >>>
> >>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> >>>> Igniters,
> >>>>
> >>>> Currently we have code sanity checks [1][2] integrated within a build
> >> task
> >>>> [3]. Do we really need to fail the build (and therefore the other
> >> tasks) if
> >>>> there is a minor flaw like a missing line at the end of a file or an
> >> unused
> >>>> import? As for me it could be separated from the build task.
> >>>>
> >>>> What do you think?
> >>>>
> >>>> [1]
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >>>> <
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >>>
> >>>> [2]
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >>>> <
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >>>
> >>>> [3]
> >>>>
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >>>> <
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >>>
> >>>>
> >>>> --
> >>>> Regards,
> >>>> Konstantin Orlov
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>> --
> >>>
> >>> Best regards,
> >>> Ivan Pavlukhin
> >>
> >>
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
>
>

-- 
Best regards,
Andrey V. Mashenkov

Re: [DISCUSSION] Separate code sanity check and build task

Posted by Nikolay Izhikov <ni...@apache.org>.
Hello, Andrey.

> I've just found that I should waste my time fixing styles in user code that (may be) reproduce some bug, just to validate the bug or fix provided by the user.

Why do you ignore code style rules while developing «quick reproducer»?

What specific code style rule is an issue for you?
If we have some rules is just a waste of the time - may be it better to remove them?

I’m ++1 to fail the build on code style errors.
Code style errors == compile errors for me.

> 8 июня 2020 г., в 16:40, Andrey Mashenkov <an...@gmail.com> написал(а):
> 
> Konstantin,
> 
> +1
> 
> I've just found that I should waste my time fixing styles in user code that
> (may be) reproduce some bug, just to validate the bug or fix provided by
> the user.
> I'm ok with the idea to block commits with style errors to master branch,
> but not for other branches\PR.
> 
> Can anybody explain why commiters should waste their time for this?
> Why we even have such a rule to fail build on TC if there is some kind of
> style error?
> It looks (like a bullshit) counterintuitive as it is still possible to
> commit to master with having style error, but impossible to just build a
> project or to run a test.
> 
> 
> On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <ko...@gridgain.com>
> wrote:
> 
>> Hi Ivan,
>> 
>> Thanks for your reply! It’s better to get an answer late than never :)
>> 
>> --
>> Regards,
>> Konstantin Orlov
>> 
>> 
>>> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com> wrote:
>>> 
>>> Hi Konstantin,
>>> 
>>> Surprisingly, I found your message in a Spam folder (gmail).
>>> 
>>> We had discussions about the subject before. The most recent one and
>>> reflecting a current state is [1]. You can find many thoughts and
>>> arguments in another discussion [2] (it might be better to start
>>> reading from a bottom).
>>> 
>>> [1]
>> https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
>>> [2]
>> http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
>>> 
>>> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
>>>> Igniters,
>>>> 
>>>> Currently we have code sanity checks [1][2] integrated within a build
>> task
>>>> [3]. Do we really need to fail the build (and therefore the other
>> tasks) if
>>>> there is a minor flaw like a missing line at the end of a file or an
>> unused
>>>> import? As for me it could be separated from the build task.
>>>> 
>>>> What do you think?
>>>> 
>>>> [1]
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
>>>> <
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
>>> 
>>>> [2]
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
>>>> <
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
>>> 
>>>> [3]
>>>> 
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
>>>> <
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
>>> 
>>>> 
>>>> --
>>>> Regards,
>>>> Konstantin Orlov
>>>> 
>>>> 
>>>> 
>>> 
>>> 
>>> --
>>> 
>>> Best regards,
>>> Ivan Pavlukhin
>> 
>> 
> 
> -- 
> Best regards,
> Andrey V. Mashenkov


Re: [DISCUSSION] Separate code sanity check and build task

Posted by Andrey Mashenkov <an...@gmail.com>.
Konstantin,

+1

I've just found that I should waste my time fixing styles in user code that
(may be) reproduce some bug, just to validate the bug or fix provided by
the user.
I'm ok with the idea to block commits with style errors to master branch,
but not for other branches\PR.

Can anybody explain why commiters should waste their time for this?
Why we even have such a rule to fail build on TC if there is some kind of
style error?
It looks (like a bullshit) counterintuitive as it is still possible to
commit to master with having style error, but impossible to just build a
project or to run a test.


On Thu, May 21, 2020 at 12:23 PM Konstantin Orlov <ko...@gridgain.com>
wrote:

> Hi Ivan,
>
> Thanks for your reply! It’s better to get an answer late than never :)
>
> --
> Regards,
> Konstantin Orlov
>
>
> > On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com> wrote:
> >
> > Hi Konstantin,
> >
> > Surprisingly, I found your message in a Spam folder (gmail).
> >
> > We had discussions about the subject before. The most recent one and
> > reflecting a current state is [1]. You can find many thoughts and
> > arguments in another discussion [2] (it might be better to start
> > reading from a bottom).
> >
> > [1]
> https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> > [2]
> http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> >
> > 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> >> Igniters,
> >>
> >> Currently we have code sanity checks [1][2] integrated within a build
> task
> >> [3]. Do we really need to fail the build (and therefore the other
> tasks) if
> >> there is a minor flaw like a missing line at the end of a file or an
> unused
> >> import? As for me it could be separated from the build task.
> >>
> >> What do you think?
> >>
> >> [1]
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >> <
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> >
> >> [2]
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >> <
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> >
> >> [3]
> >>
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >> <
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> >
> >>
> >> --
> >> Regards,
> >> Konstantin Orlov
> >>
> >>
> >>
> >
> >
> > --
> >
> > Best regards,
> > Ivan Pavlukhin
>
>

-- 
Best regards,
Andrey V. Mashenkov

Re: [DISCUSSION] Separate code sanity check and build task

Posted by Konstantin Orlov <ko...@gridgain.com>.
Hi Ivan,

Thanks for your reply! It’s better to get an answer late than never :)

-- 
Regards,
Konstantin Orlov


> On 18 May 2020, at 09:04, Ivan Pavlukhin <vo...@gmail.com> wrote:
> 
> Hi Konstantin,
> 
> Surprisingly, I found your message in a Spam folder (gmail).
> 
> We had discussions about the subject before. The most recent one and
> reflecting a current state is [1]. You can find many thoughts and
> arguments in another discussion [2] (it might be better to start
> reading from a bottom).
> 
> [1] https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
> [2] http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297
> 
> 2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
>> Igniters,
>> 
>> Currently we have code sanity checks [1][2] integrated within a build task
>> [3]. Do we really need to fail the build (and therefore the other tasks) if
>> there is a minor flaw like a missing line at the end of a file or an unused
>> import? As for me it could be separated from the build task.
>> 
>> What do you think?
>> 
>> [1]
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
>> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle>
>> [2]
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
>> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders>
>> [3]
>> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
>> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite>
>> 
>> --
>> Regards,
>> Konstantin Orlov
>> 
>> 
>> 
> 
> 
> -- 
> 
> Best regards,
> Ivan Pavlukhin


Re: [DISCUSSION] Separate code sanity check and build task

Posted by Ivan Pavlukhin <vo...@gmail.com>.
Hi Konstantin,

Surprisingly, I found your message in a Spam folder (gmail).

We had discussions about the subject before. The most recent one and
reflecting a current state is [1]. You can find many thoughts and
arguments in another discussion [2] (it might be better to start
reading from a bottom).

[1] https://lists.apache.org/thread.html/6995a4e789117ba3f5577651866cfa99a6ffcc208cf60330d17d5a48%40%3Cdev.ignite.apache.org%3E
[2] http://apache-ignite-developers.2346864.n4.nabble.com/Code-inspection-td27709i80.html#a41297

2020-04-20 11:00 GMT+03:00, Konstantin Orlov <ko...@gridgain.com>:
> Igniters,
>
> Currently we have code sanity checks [1][2] integrated within a build task
> [3]. Do we really need to fail the build (and therefore the other tasks) if
> there is a minor flaw like a missing line at the end of a file or an unused
> import? As for me it could be separated from the build task.
>
> What do you think?
>
> [1]
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle
> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_CheckCodeStyle>
> [2]
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders
> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_LicensesHeaders>
> [3]
> https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite
> <https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_BuildApacheIgnite>
>
> --
> Regards,
> Konstantin Orlov
>
>
>


-- 

Best regards,
Ivan Pavlukhin