linting issues on ‘trunk’, ‘gradlew check’ fails.

[Mathieu Lirzin]

I recently pulled the latest commit, and I noticed that latest commits
have introducted linting issues.

Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.

Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
you have written to conform to OFBiz coding style ?

Thanks.

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[Jacques Le Roux]

I checked, I'm not concerned :)

But I wonder if it was really OK at a521de9ad8a0b5a0f9ceadab348c46d7961ff89a. Because the 1st error is

 > Task :compileJava
C:\projectsASF\Git\ofbiz-framework\plugins\bi\src\main\java\org\apache\ofbiz\bi\util\DimensionServices.java:68: error: cannot find symbol
         List<String> naturalKeyFields = UtilGenerics.checkList(context.get("naturalKeyFields"), String.class);
                                                     ^

And this class has not been changed for almost 3 years. Same for all classes in birt plugin, and I guess in all plugins components.

There are 38 errors and 67 warnings. Seems most issues are related with casting. This was when using
https://gitbox.apache.org/repos/asf/ofbiz-plugins.git (up to date) directly inside https://gitbox.apache.org/repos/asf/ofbiz-framework.git as plugins dir.

I then tried using "gradlew pullAllPluginsSource" and got 37862 errors. I don't understand the difference because with "gradlew
pullAllPluginsSource"even not modified "framework" classes are concerned by errors when in the 1st case it's only plugins classes

I did not went further...

Jacques

Le 23/11/2019 à 17:07, Mathieu Lirzin a écrit :

> I recently pulled the latest commit, and I noticed that latest commits
> have introducted linting issues.
>
> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>
> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
> you have written to conform to OFBiz coding style ?
>
> Thanks.
>

[Jacques Le Roux]

Curious I compared the plugins from https://gitbox.apache.org/repos/asf/ofbiz-plugins.git and https://github.com/apache/ofbiz-plugins/trunk : no
differences :-\

Le 23/11/2019 à 21:50, Jacques Le Roux a écrit :

> I checked, I'm not concerned :)
>
> But I wonder if it was really OK at a521de9ad8a0b5a0f9ceadab348c46d7961ff89a. Because the 1st error is
>
> > Task :compileJava
> C:\projectsASF\Git\ofbiz-framework\plugins\bi\src\main\java\org\apache\ofbiz\bi\util\DimensionServices.java:68: error: cannot find symbol
>         List<String> naturalKeyFields = UtilGenerics.checkList(context.get("naturalKeyFields"), String.class);
>                                                     ^
>
> And this class has not been changed for almost 3 years. Same for all classes in birt plugin, and I guess in all plugins components.
>
> There are 38 errors and 67 warnings. Seems most issues are related with casting. This was when using
> https://gitbox.apache.org/repos/asf/ofbiz-plugins.git (up to date) directly inside https://gitbox.apache.org/repos/asf/ofbiz-framework.git as
> plugins dir.
>
> I then tried using "gradlew pullAllPluginsSource" and got 37862 errors. I don't understand the difference because with "gradlew
> pullAllPluginsSource"even not modified "framework" classes are concerned by errors when in the 1st case it's only plugins classes
>
> I did not went further...
>
> Jacques
>
> Le 23/11/2019 à 17:07, Mathieu Lirzin a écrit :
>> I recently pulled the latest commit, and I noticed that latest commits
>> have introducted linting issues.
>>
>> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>>
>> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
>> you have written to conform to OFBiz coding style ?
>>
>> Thanks.
>>
>

[Pawan Verma]

Done from my side, Apologies for the inconvenience.

--
Thanks & Regards
Pawan Verma
Technical Consultant
*HotWax Systems*
*Enterprise open source experts*
http://www.hotwaxsystems.com


On Sun, Nov 24, 2019 at 2:36 AM Jacques Le Roux <
[hidden email]> wrote:

> Curious I compared the plugins from
> https://gitbox.apache.org/repos/asf/ofbiz-plugins.git and
> https://github.com/apache/ofbiz-plugins/trunk : no
> differences :-\
>
> Le 23/11/2019 à 21:50, Jacques Le Roux a écrit :
> > I checked, I'm not concerned :)
> >
> > But I wonder if it was really OK at
> a521de9ad8a0b5a0f9ceadab348c46d7961ff89a. Because the 1st error is
> >
> > > Task :compileJava
> >
> C:\projectsASF\Git\ofbiz-framework\plugins\bi\src\main\java\org\apache\ofbiz\bi\util\DimensionServices.java:68:
> error: cannot find symbol
> >         List<String> naturalKeyFields =
> UtilGenerics.checkList(context.get("naturalKeyFields"), String.class);
> >                                                     ^
> >
> > And this class has not been changed for almost 3 years. Same for all
> classes in birt plugin, and I guess in all plugins components.
> >
> > There are 38 errors and 67 warnings. Seems most issues are related with
> casting. This was when using
> > https://gitbox.apache.org/repos/asf/ofbiz-plugins.git (up to date)
> directly inside https://gitbox.apache.org/repos/asf/ofbiz-framework.git
> as
> > plugins dir.
> >
> > I then tried using "gradlew pullAllPluginsSource" and got 37862 errors.
> I don't understand the difference because with "gradlew
> > pullAllPluginsSource"even not modified "framework" classes are concerned
> by errors when in the 1st case it's only plugins classes
> >
> > I did not went further...
> >
> > Jacques
> >
> > Le 23/11/2019 à 17:07, Mathieu Lirzin a écrit :
> >> I recently pulled the latest commit, and I noticed that latest commits
> >> have introducted linting issues.
> >>
> >> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
> >>
> >> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
> >> you have written to conform to OFBiz coding style ?
> >>
> >> Thanks.
> >>
> >
>

[Nicolas Malin-2]

I run the check with success from my part but with 34563 errors, I
suppose that it's not only mine :)

Nicolas

On 23/11/2019 17:07, Mathieu Lirzin wrote:

> I recently pulled the latest commit, and I noticed that latest commits
> have introducted linting issues.
>
> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>
> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
> you have written to conform to OFBiz coding style ?
>
> Thanks.
>

[Jacques Le Roux]

Here with Gitbox plugins I have 37790 errors and when using pullAllPluginsSource (ie using svn with Github) I have 37857 errors

In both case it's more than /tasks.checkstyleMain.maxErrors = 37779/

I don't understand why we have all these differences (34563 errors for you Nicolas) when we are supposed to use the same "trunk HEAD"

Jacques

Le 25/11/2019 à 09:17, Nicolas Malin a écrit :

> I run the check with success from my part but with 34563 errors, I suppose that it's not only mine :)
>
> Nicolas
>
> On 23/11/2019 17:07, Mathieu Lirzin wrote:
>> I recently pulled the latest commit, and I noticed that latest commits
>> have introducted linting issues.
>>
>> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>>
>> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
>> you have written to conform to OFBiz coding style ?
>>
>> Thanks.
>>
>

[Jacques Le Roux]

Since it depends on the good will of scrutinizer/s and varies in errors depending on context, I suggest that we add a "gradlew check" step in our
Builbot trunk framework builder.

Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443 for that...

Jacques

Le 25/11/2019 à 11:18, Jacques Le Roux a écrit :

> Here with Gitbox plugins I have 37790 errors and when using pullAllPluginsSource (ie using svn with Github) I have 37857 errors
>
> In both case it's more than /tasks.checkstyleMain.maxErrors = 37779/
>
> I don't understand why we have all these differences (34563 errors for you Nicolas) when we are supposed to use the same "trunk HEAD"
>
> Jacques
>
> Le 25/11/2019 à 09:17, Nicolas Malin a écrit :
>> I run the check with success from my part but with 34563 errors, I suppose that it's not only mine :)
>>
>> Nicolas
>>
>> On 23/11/2019 17:07, Mathieu Lirzin wrote:
>>> I recently pulled the latest commit, and I noticed that latest commits
>>> have introducted linting issues.
>>>
>>> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>>>
>>> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
>>> you have written to conform to OFBiz coding style ?
>>>
>>> Thanks.
>>>
>>
>

[Mathieu Lirzin]

Jacques Le Roux <[hidden email]> writes:

> Since it depends on the good will of scrutinizer/s and varies in
> errors depending on context, I suggest that we add a "gradlew check"
> step in our Builbot trunk framework builder.
>
> Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443 for that...

Excellent idea.

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[Jacques Le Roux]

To make things absolutely clear, in Buildbot we use pullAllPluginsSource. So "gradlew check" will include the plugins

I created OFBIZ-11299 for that

Jacques

Le 26/11/2019 à 18:04, Mathieu Lirzin a écrit :
> Jacques Le Roux <[hidden email]> writes:
>
>> Since it depends on the good will of scrutinizer/s and varies in
>> errors depending on context, I suggest that we add a "gradlew check"
>> step in our Builbot trunk framework builder.
>>
>> Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443 for that...
> Excellent idea.
>

[Jacques Le Roux]

Done, with that something changed.

Before https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins was only failing with tests. Now it fails also with lint (as you can see[1] tests are
OK). The lint result is there[2]

It's not the case for https://ci.apache.org/builders/ofbizTrunkFramework where the check task is not run.

[1] https://ci.apache.org/projects/ofbiz/logs/trunk/plugins/html/
[2] https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html

Jacques

Le 27/11/2019 à 09:35, Jacques Le Roux a écrit :

> To make things absolutely clear, in Buildbot we use pullAllPluginsSource. So "gradlew check" will include the plugins
>
> I created OFBIZ-11299 for that
>
> Jacques
>
> Le 26/11/2019 à 18:04, Mathieu Lirzin a écrit :
>> Jacques Le Roux <[hidden email]> writes:
>>
>>> Since it depends on the good will of scrutinizer/s and varies in
>>> errors depending on context, I suggest that we add a "gradlew check"
>>> step in our Builbot trunk framework builder.
>>>
>>> Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443 for
>>> that...
>> Excellent idea.
>>
>

[Jacques Le Roux]

Hi Nicolas,

Did you include plugins? This seems low compared to current 37776

Thanks

Jacques

Le 25/11/2019 à 09:17, Nicolas Malin a écrit :

> I run the check with success from my part but with 34563 errors, I suppose that it's not only mine :)
>
> Nicolas
>
> On 23/11/2019 17:07, Mathieu Lirzin wrote:
>> I recently pulled the latest commit, and I noticed that latest commits
>> have introducted linting issues.
>>
>> Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails.
>>
>> Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code
>> you have written to conform to OFBiz coding style ?
>>
>> Thanks.
>>
>

[Jacques Le Roux]

Hi Nicolas, Pawan,

Mathieu said that the lint errors which remains are yours: https://markmail.org/message/2ilvc5swhem4fuxn. I tend to think so :)

The ofbizTrunkFrameworkPlugins build will always fail as long as we have not cured those.

But it's now "hard" to exactly know from where they come.

I think we should rely on a  Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors

So every committer would have it installed locally and the problem would be gone \o/

What do people think?

Jacques


Le 29/11/2019 à 08:18, Jacques Le Roux a écrit :

> Done, with that something changed.
>
> Before https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins was only failing with tests. Now it fails also with lint (as you can see[1] tests
> are OK). The lint result is there[2]
>
> It's not the case for https://ci.apache.org/builders/ofbizTrunkFramework where the check task is not run.
>
> [1] https://ci.apache.org/projects/ofbiz/logs/trunk/plugins/html/
> [2] https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html
>
> Jacques
>
> Le 27/11/2019 à 09:35, Jacques Le Roux a écrit :
>> To make things absolutely clear, in Buildbot we use pullAllPluginsSource. So "gradlew check" will include the plugins
>>
>> I created OFBIZ-11299 for that
>>
>> Jacques
>>
>> Le 26/11/2019 à 18:04, Mathieu Lirzin a écrit :
>>> Jacques Le Roux <[hidden email]> writes:
>>>
>>>> Since it depends on the good will of scrutinizer/s and varies in
>>>> errors depending on context, I suggest that we add a "gradlew check"
>>>> step in our Builbot trunk framework builder.
>>>>
>>>> Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443 for
>>>> that...
>>> Excellent idea.
>>>
>>
>

[Jacques Le Roux]

Hi Nicolas, Pawan,

With OFBIZ-11278 Mathieu reduced maxErrors to 37769, so it's not a worry for you anymore :)

Jacques

Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :

> Hi Nicolas, Pawan,
>
> Mathieu said that the lint errors which remains are yours: https://markmail.org/message/2ilvc5swhem4fuxn. I tend to think so :)
>
> The ofbizTrunkFrameworkPlugins build will always fail as long as we have not cured those.
>
> But it's now "hard" to exactly know from where they come.
>
> I think we should rely on a  Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>
> So every committer would have it installed locally and the problem would be gone \o/
>
> What do people think?
>
> Jacques
>
>
> Le 29/11/2019 à 08:18, Jacques Le Roux a écrit :
>> Done, with that something changed.
>>
>> Before https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins was only failing with tests. Now it fails also with lint (as you can see[1] tests
>> are OK). The lint result is there[2]
>>
>> It's not the case for https://ci.apache.org/builders/ofbizTrunkFramework where the check task is not run.
>>
>> [1] https://ci.apache.org/projects/ofbiz/logs/trunk/plugins/html/
>> [2] https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html
>>
>> Jacques
>>
>> Le 27/11/2019 à 09:35, Jacques Le Roux a écrit :
>>> To make things absolutely clear, in Buildbot we use pullAllPluginsSource. So "gradlew check" will include the plugins
>>>
>>> I created OFBIZ-11299 for that
>>>
>>> Jacques
>>>
>>> Le 26/11/2019 à 18:04, Mathieu Lirzin a écrit :
>>>> Jacques Le Roux <[hidden email]> writes:
>>>>
>>>>> Since it depends on the good will of scrutinizer/s and varies in
>>>>> errors depending on context, I suggest that we add a "gradlew check"
>>>>> step in our Builbot trunk framework builder.
>>>>>
>>>>> Then everyone will be aware of this feature and we will be able to adjust the maxErrors as needed. Actually I'm still waiting for INFRA-19443
>>>>> for that...
>>>> Excellent idea.
>>>>
>>>
>>
>

[Jacques Le Roux]

Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
> I think we should rely on a  Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>
> So every committer would have it installed locally and the problem would be gone \o/
>
> What do people think?
>
> Jacques
I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that

Jacques

[Nicolas Malin-2]

I haven't plugin installed, only famework.

Thanks for your sharing a will use that.

Nicolas

On 02/12/2019 17:32, Jacques Le Roux wrote:

> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
>> I think we should rely on a  Checkstyle pre-commit hook:
>> https://gist.github.com/davetron5000/37350 to complement
>> tasks.checkstyleMain.maxErrors
>>
>> So every committer would have it installed locally and the problem
>> would be gone \o/
>>
>> What do people think?
>>
>> Jacques
> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
>
> Jacques
>
>

[Jacques Le Roux]

Hi Nicolas,

Great, I think we should use it as a team, nobody against?

Jacques

Le 03/12/2019 à 09:21, Nicolas Malin a écrit :

> I haven't plugin installed, only famework.
>
> Thanks for your sharing a will use that.
>
> Nicolas
>
> On 02/12/2019 17:32, Jacques Le Roux wrote:
>> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
>>> I think we should rely on a  Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>>>
>>> So every committer would have it installed locally and the problem would be gone \o/
>>>
>>> What do people think?
>>>
>>> Jacques
>> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
>>
>> Jacques
>>
>>
>

[Jacques Le Roux]

Hi All,

Since then we also created INFRA-20314

At OFBIZ-11304 Aditya suggested something else. To use a Gradle plugin to add pre-commit hook.

I tested it, it does not prevent a committer to push changes even if they increase the number of check style issues.

I'd like to know what the community prefers:

  * Prevent a committer to push changes even if they increase the number of check style issues. This implies to implement INFRA-20314
  * Allows a committer to push changes even if they increase the number of check style issues. Then we could use OFBIZ-11304 only.

What do you think? If we can get to a consensus we might start a vote

Thanks

Jacques

Le 03/12/2019 à 09:42, Jacques Le Roux a écrit :

> Hi Nicolas,
>
> Great, I think we should use it as a team, nobody against?
>
> Jacques
>
> Le 03/12/2019 à 09:21, Nicolas Malin a écrit :
>> I haven't plugin installed, only famework.
>>
>> Thanks for your sharing a will use that.
>>
>> Nicolas
>>
>> On 02/12/2019 17:32, Jacques Le Roux wrote:
>>> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
>>>> I think we should rely on a Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>>>>
>>>> So every committer would have it installed locally and the problem would be gone \o/
>>>>
>>>> What do people think?
>>>>
>>>> Jacques
>>> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
>>>
>>> Jacques
>>>
>>>
>>
>

[James Yong-2]

Hi all,

+1 to INFRA-20314.

Committer should install a checkstyle plugin in the IDE pointing to our checkstyle.xml.
This helps to highlight Lint errors so that style issues can be corrected early.

Regards,
James

On 2020/06/05 17:30:18, Jacques Le Roux <[hidden email]> wrote:

> Hi All,
>
> Since then we also created INFRA-20314
>
> At OFBIZ-11304 Aditya suggested something else. To use a Gradle plugin to add pre-commit hook.
>
> I tested it, it does not prevent a committer to push changes even if they increase the number of check style issues.
>
> I'd like to know what the community prefers:
>
>   * Prevent a committer to push changes even if they increase the number of check style issues. This implies to implement INFRA-20314
>   * Allows a committer to push changes even if they increase the number of check style issues. Then we could use OFBIZ-11304 only.
>
> What do you think? If we can get to a consensus we might start a vote
>
> Thanks
>
> Jacques
>
> Le 03/12/2019 à 09:42, Jacques Le Roux a écrit :
> > Hi Nicolas,
> >
> > Great, I think we should use it as a team, nobody against?
> >
> > Jacques
> >
> > Le 03/12/2019 à 09:21, Nicolas Malin a écrit :
> >> I haven't plugin installed, only famework.
> >>
> >> Thanks for your sharing a will use that.
> >>
> >> Nicolas
> >>
> >> On 02/12/2019 17:32, Jacques Le Roux wrote:
> >>> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
> >>>> I think we should rely on a Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
> >>>>
> >>>> So every committer would have it installed locally and the problem would be gone \o/
> >>>>
> >>>> What do people think?
> >>>>
> >>>> Jacques
> >>> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
> >>>
> >>> Jacques
> >>>
> >>>
> >>
> >
>

[Jacques Le Roux]

Hi,

As you can see at OFBIZ-11304, Aditya increased the possibility of the pre-commit hook.

I think we can use that right now, and see if ever we get issues which I doubt after testing.

Thanks

Le 06/06/2020 à 05:08, James Yong a écrit :

> Hi all,
>
> +1 to INFRA-20314.
>
> Committer should install a checkstyle plugin in the IDE pointing to our checkstyle.xml.
> This helps to highlight Lint errors so that style issues can be corrected early.
>
> Regards,
> James
>
> On 2020/06/05 17:30:18, Jacques Le Roux <[hidden email]> wrote:
>> Hi All,
>>
>> Since then we also created INFRA-20314
>>
>> At OFBIZ-11304 Aditya suggested something else. To use a Gradle plugin to add pre-commit hook.
>>
>> I tested it, it does not prevent a committer to push changes even if they increase the number of check style issues.
>>
>> I'd like to know what the community prefers:
>>
>>    * Prevent a committer to push changes even if they increase the number of check style issues. This implies to implement INFRA-20314
>>    * Allows a committer to push changes even if they increase the number of check style issues. Then we could use OFBIZ-11304 only.
>>
>> What do you think? If we can get to a consensus we might start a vote
>>
>> Thanks
>>
>> Jacques
>>
>> Le 03/12/2019 à 09:42, Jacques Le Roux a écrit :
>>> Hi Nicolas,
>>>
>>> Great, I think we should use it as a team, nobody against?
>>>
>>> Jacques
>>>
>>> Le 03/12/2019 à 09:21, Nicolas Malin a écrit :
>>>> I haven't plugin installed, only famework.
>>>>
>>>> Thanks for your sharing a will use that.
>>>>
>>>> Nicolas
>>>>
>>>> On 02/12/2019 17:32, Jacques Le Roux wrote:
>>>>> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
>>>>>> I think we should rely on a Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>>>>>>
>>>>>> So every committer would have it installed locally and the problem would be gone \o/
>>>>>>
>>>>>> What do people think?
>>>>>>
>>>>>> Jacques
>>>>> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
>>>>>
>>>>> Jacques
>>>>>
>>>>>

[Jacques Le Roux]

Hi All,

I have finally pushed a pre-push hook rather than a pre-commit hook.
I  noticed that this does not clear the existing pre-commit hook in .git\hooks. You have to clear it by hand.

But anyway, I believe we should rather use a webhook as proposed by Infra. This for at least 3 reasons:

 1. Handles PRs and not only local commits
 2. Minimise the CPU usage locally
 3. Simplify usage for custom projects

The next step is to comment out the feature and let people know it exists in documentation.
This in order for committers who prefer to check before pushing but tend to forget.
And for possibly interested custom projects.

I'll do so and ask Infra to create a webhook (INFRA-20314) after the weekend if nobody is against

Jacques

Le 12/06/2020 à 13:41, Jacques Le Roux a écrit :

> Hi,
>
> As you can see at OFBIZ-11304, Aditya increased the possibility of the pre-commit hook.
>
> I think we can use that right now, and see if ever we get issues which I doubt after testing.
>
> Thanks
>
> Le 06/06/2020 à 05:08, James Yong a écrit :
>> Hi all,
>>
>> +1 to INFRA-20314.
>>
>> Committer should install a checkstyle plugin in the IDE pointing to our checkstyle.xml.
>> This helps to highlight Lint errors so that style issues can be corrected early.
>>
>> Regards,
>> James
>>
>> On 2020/06/05 17:30:18, Jacques Le Roux <[hidden email]> wrote:
>>> Hi All,
>>>
>>> Since then we also created INFRA-20314
>>>
>>> At OFBIZ-11304 Aditya suggested something else. To use a Gradle plugin to add pre-commit hook.
>>>
>>> I tested it, it does not prevent a committer to push changes even if they increase the number of check style issues.
>>>
>>> I'd like to know what the community prefers:
>>>
>>>    * Prevent a committer to push changes even if they increase the number of check style issues. This implies to implement INFRA-20314
>>>    * Allows a committer to push changes even if they increase the number of check style issues. Then we could use OFBIZ-11304 only.
>>>
>>> What do you think? If we can get to a consensus we might start a vote
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>> Le 03/12/2019 à 09:42, Jacques Le Roux a écrit :
>>>> Hi Nicolas,
>>>>
>>>> Great, I think we should use it as a team, nobody against?
>>>>
>>>> Jacques
>>>>
>>>> Le 03/12/2019 à 09:21, Nicolas Malin a écrit :
>>>>> I haven't plugin installed, only famework.
>>>>>
>>>>> Thanks for your sharing a will use that.
>>>>>
>>>>> Nicolas
>>>>>
>>>>> On 02/12/2019 17:32, Jacques Le Roux wrote:
>>>>>> Le 30/11/2019 à 08:58, Jacques Le Roux a écrit :
>>>>>>> I think we should rely on a Checkstyle pre-commit hook: https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors
>>>>>>>
>>>>>>> So every committer would have it installed locally and the problem would be gone \o/
>>>>>>>
>>>>>>> What do people think?
>>>>>>>
>>>>>>> Jacques
>>>>>> I created https://issues.apache.org/jira/browse/OFBIZ-11304 for that
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>