You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Michael Brohl <mi...@ecomify.de> on 2021/02/03 16:59:16 UTC

Checkstyle pre-push hook not used when merging PR's

Hi all,

I just noticed the following behaviour while trying to commit my work 
for https://issues.apache.org/jira/browse/OFBIZ-12165 .

I only changed the Tomcat/Catalina version in the build.gradle file and 
got the following error from the checkstyle plugin:

Execution failed for task ':checkstyleMain'.
 > Checkstyle rule violations were found. See the report at: 
file:///Users/mbrohl/Projects/apache-ofbiz/ofbiz-framework/build/reports/checkstyle/main.html
   Checkstyle files with violations: 132
   Checkstyle violations by severity: [error:329]

This is correct, the configured count is 278 and there are 329 errors found.

Today, I merged several pull requests using GitHub, everything goes to 
the codebase without problems. My change to build.gradle cannot be 
responsible for the checkstyle errors so I assume that the pre-push hook 
does not get fired (because it's a merge not a push). Those checkstyle 
errors must have been introduced by the latest commits from the pull 
requests.

I then noticed that our forked repository here at ecomify does not 
contain the pre-push hook. Which is reasonable because the .git folder 
is not versioned and the hook is not automatically received by forked 
repositories, afaik.

So I think we should find a way to deploy the hooks to the user's local 
repository to make sure they are used there also. Else we would always 
chase after newly introduced checkstyle problems, especially if we use 
pull requests.

I found a solution here: 
https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/ 
(at the bottom of the page). This must be adapted for Gradle to be 
dependend of a build.

This would keep our hooks versioned in the repository and would 
automatically install the hooks in the local repository.

What do you think?

Regards,

Michael Brohl

ecomify GmbH - www.ecomify.de



Re: Checkstyle pre-push hook not used when merging PR's

Posted by Michael Brohl <mi...@ecomify.de>.
I just found that the error count in my local repository was higher 
because of integrated plugins so it seems the codebase is clean.

To my understanding, the described gap with the missing hooks on local 
repositories still exists.

Michael


Am 03.02.21 um 17:59 schrieb Michael Brohl:
> Hi all,
>
> I just noticed the following behaviour while trying to commit my work 
> for https://issues.apache.org/jira/browse/OFBIZ-12165 .
>
> I only changed the Tomcat/Catalina version in the build.gradle file 
> and got the following error from the checkstyle plugin:
>
> Execution failed for task ':checkstyleMain'.
> > Checkstyle rule violations were found. See the report at: 
> file:///Users/mbrohl/Projects/apache-ofbiz/ofbiz-framework/build/reports/checkstyle/main.html
>   Checkstyle files with violations: 132
>   Checkstyle violations by severity: [error:329]
>
> This is correct, the configured count is 278 and there are 329 errors 
> found.
>
> Today, I merged several pull requests using GitHub, everything goes to 
> the codebase without problems. My change to build.gradle cannot be 
> responsible for the checkstyle errors so I assume that the pre-push 
> hook does not get fired (because it's a merge not a push). Those 
> checkstyle errors must have been introduced by the latest commits from 
> the pull requests.
>
> I then noticed that our forked repository here at ecomify does not 
> contain the pre-push hook. Which is reasonable because the .git folder 
> is not versioned and the hook is not automatically received by forked 
> repositories, afaik.
>
> So I think we should find a way to deploy the hooks to the user's 
> local repository to make sure they are used there also. Else we would 
> always chase after newly introduced checkstyle problems, especially if 
> we use pull requests.
>
> I found a solution here: 
> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/ 
> (at the bottom of the page). This must be adapted for Gradle to be 
> dependend of a build.
>
> This would keep our hooks versioned in the repository and would 
> automatically install the hooks in the local repository.
>
> What do you think?
>
> Regards,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks Michael,

I'll have a look too, but not yet. I think there are some low hanging fruits.

Jacques

Le 05/02/2021 à 15:33, Michael Brohl a écrit :
> Ah, thanks Jacques.
>
> I have just added a new subtask to the checkstyle Jira to fix more errors: https://issues.apache.org/jira/browse/OFBIZ-12169
>
> Regards,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
> Am 05.02.21 um 12:33 schrieb Jacques Le Roux:
>> No worries, yes it's there
>> https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html
>>
>> Le 05/02/2021 à 11:26, Michael Brohl a écrit :
>>> Sorry Jacques, I missed this.
>>>
>>> Is there a way to look at the artifacts produced by the buildbot builds? In this case, how can I access
>>>
>>> /home/buildslave/slave/ofbizTrunkFrameworkPlugins/build/build/reports/checkstyle/main.html
>>>
>>> through the browser to see where the problems are?
>>>
>>> Thanks for taking care,
>>>
>>> Michael Brohl
>>>
>>> ecomify GmbH - www.ecomify.de
>>>
>>>
>>> Am 05.02.21 um 11:15 schrieb Jacques Le Roux:
>>>> OK, I need it right now, so I'm taking care of it, not a big deal anyway
>>>>
>>>> Le 05/02/2021 à 09:57, Jacques Le Roux a écrit :
>>>>> Hi Michael,
>>>>>
>>>>> It's better but we have still a small issue. The problem is not this small issue but that it prevents others (not only check issues) to show 
>>>>> until it's fixed...
>>>>>
>>>>> TIA to care
>>>>>
>>>>> Jacques
>>>>>
>>>>> Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>>>>>>
>>>>>> In the meantime you need to fix the checkstyle issues put in with
>>>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>>>>>> details here:
>>>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>>>>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Michael Brohl <mi...@ecomify.de>.
Ah, thanks Jacques.

I have just added a new subtask to the checkstyle Jira to fix more 
errors: https://issues.apache.org/jira/browse/OFBIZ-12169

Regards,

Michael Brohl

ecomify GmbH - www.ecomify.de


Am 05.02.21 um 12:33 schrieb Jacques Le Roux:
> No worries, yes it's there
> https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html
>
> Le 05/02/2021 à 11:26, Michael Brohl a écrit :
>> Sorry Jacques, I missed this.
>>
>> Is there a way to look at the artifacts produced by the buildbot 
>> builds? In this case, how can I access
>>
>> /home/buildslave/slave/ofbizTrunkFrameworkPlugins/build/build/reports/checkstyle/main.html 
>>
>>
>> through the browser to see where the problems are?
>>
>> Thanks for taking care,
>>
>> Michael Brohl
>>
>> ecomify GmbH - www.ecomify.de
>>
>>
>> Am 05.02.21 um 11:15 schrieb Jacques Le Roux:
>>> OK, I need it right now, so I'm taking care of it, not a big deal 
>>> anyway
>>>
>>> Le 05/02/2021 à 09:57, Jacques Le Roux a écrit :
>>>> Hi Michael,
>>>>
>>>> It's better but we have still a small issue. The problem is not 
>>>> this small issue but that it prevents others (not only check 
>>>> issues) to show until it's fixed...
>>>>
>>>> TIA to care
>>>>
>>>> Jacques
>>>>
>>>> Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>>>>>
>>>>> In the meantime you need to fix the checkstyle issues put in with
>>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>>>>> details here:
>>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio 
>>>>>
>>>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Jacques Le Roux <ja...@les7arts.com>.
No worries, yes it's there
https://ci.apache.org/projects/ofbiz/logs/trunk/checkstyle.html

Le 05/02/2021 à 11:26, Michael Brohl a écrit :
> Sorry Jacques, I missed this.
>
> Is there a way to look at the artifacts produced by the buildbot builds? In this case, how can I access
>
> /home/buildslave/slave/ofbizTrunkFrameworkPlugins/build/build/reports/checkstyle/main.html
>
> through the browser to see where the problems are?
>
> Thanks for taking care,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
> Am 05.02.21 um 11:15 schrieb Jacques Le Roux:
>> OK, I need it right now, so I'm taking care of it, not a big deal anyway
>>
>> Le 05/02/2021 à 09:57, Jacques Le Roux a écrit :
>>> Hi Michael,
>>>
>>> It's better but we have still a small issue. The problem is not this small issue but that it prevents others (not only check issues) to show until 
>>> it's fixed...
>>>
>>> TIA to care
>>>
>>> Jacques
>>>
>>> Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>>>>
>>>> In the meantime you need to fix the checkstyle issues put in with
>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>>>> details here:
>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Michael Brohl <mi...@ecomify.de>.
Sorry Jacques, I missed this.

Is there a way to look at the artifacts produced by the buildbot builds? 
In this case, how can I access

/home/buildslave/slave/ofbizTrunkFrameworkPlugins/build/build/reports/checkstyle/main.html

through the browser to see where the problems are?

Thanks for taking care,

Michael Brohl

ecomify GmbH - www.ecomify.de


Am 05.02.21 um 11:15 schrieb Jacques Le Roux:
> OK, I need it right now, so I'm taking care of it, not a big deal anyway
>
> Le 05/02/2021 à 09:57, Jacques Le Roux a écrit :
>> Hi Michael,
>>
>> It's better but we have still a small issue. The problem is not this 
>> small issue but that it prevents others (not only check issues) to 
>> show until it's fixed...
>>
>> TIA to care
>>
>> Jacques
>>
>> Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>>>
>>> In the meantime you need to fix the checkstyle issues put in with
>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>>> details here:
>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio 
>>>
>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Jacques Le Roux <ja...@les7arts.com>.
OK, I need it right now, so I'm taking care of it, not a big deal anyway

Le 05/02/2021 à 09:57, Jacques Le Roux a écrit :
> Hi Michael,
>
> It's better but we have still a small issue. The problem is not this small issue but that it prevents others (not only check issues) to show until 
> it's fixed...
>
> TIA to care
>
> Jacques
>
> Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>>
>> In the meantime you need to fix the checkstyle issues put in with
>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>> details here:
>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Michael,

It's better but we have still a small issue. The problem is not this small issue but that it prevents others (not only check issues) to show until 
it's fixed...

TIA to care

Jacques

Le 04/02/2021 à 09:04, Jacques Le Roux a écrit :
>
> In the meantime you need to fix the checkstyle issues put in with
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
> details here:
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio


Re: Checkstyle pre-push hook not used when merging PR's

Posted by Girish Vasmatkar <gi...@hotwaxsystems.com>.
Hi Michael -

It is probably not possible to execute the hooks automatically. There are
security implications to it based on a bit of research as the hooks run on
the client side.

I think sharing is not an issue because the gradle plug-in takes care of
generating the hook with any gradle command run. The closest thing I found
is the following and there will be some manual steps involved -

https://github.com/git-hook/post-clone

This tries to create sort of post-clone hook by using
https://git-scm.com/docs/git-init#_template_directory

Best Regards,
Girish





On Thu, Feb 4, 2021 at 7:33 PM Michael Brohl <mi...@ecomify.de>
wrote:

> Thanks a lot for the comprehensive informations, Aditya!
>
> I somehow missed this activity or simply forgot.
>
> The hooks are not generated by default, during the build, right? If we
> would do this automatically we could close this gap (which is minor
> because the build does the checks anyway).
>
> For plugins, this is indeed a problem because we do not have a build
> mechanism there. We could check through the framework build with
> integrated plugins, which is the only way to automatically test the
> plugins anyway. The problem would be the counter which is targeted at
> the framework code.
>
> Is it possible to specify folder groups or packages to check separately
> so that the framework build could check against one counter for the
> framework code and another counter for the plugins code?
>
> Thanks,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
> Am 04.02.21 um 13:00 schrieb Aditya Sharma:
> > Hi Michael,
> >
> > With OFBIZ-11304[1], we introduced a Gradle plugin
> > git-hooks-gradle-plugin[2]. The plugin generates a pre-push hook on the
> > developer's local. I think as the plugin persists it should work along
> with
> > the forks too. Along with that, we used GitHub action that runs
> > checkstyleMain task on PRs as discussed here[3].
> >
> > We only introduced these with ofbiz-framework repository and not with
> > ofbiz-plugins. We do not have Gradle in place for the ofbiz-plugin
> > repository so the same arrangement cannot be done and as discussed
> here[4]
> > it's checked while checking the trunk in Buildbot.
> >
> > The new checkstyle issues must be introduced from the plugins I guess.
> >
> >
> > 1. https://issues.apache.org/jira/browse/OFBIZ-11304
> > 2. https://github.com/jakemarsden/git-hooks-gradle-plugin
> > 3.
> >
> https://issues.apache.org/jira/browse/OFBIZ-11304?focusedCommentId=17138097&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17138097
> > 4.
> >
> https://issues.apache.org/jira/browse/OFBIZ-11251?focusedCommentId=17183889&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17183889
> >
> > Thanks and regards,
> > Aditya Sharma
> >
> > On Thu, Feb 4, 2021 at 4:33 PM Michael Brohl <mi...@ecomify.de>
> > wrote:
> >
> >> Hi Girish,
> >>
> >> which githook plugin are you using? I found several, not sure which one
> >> to choose.
> >>
> >> It seems that they do exactly what I described, just have to check. If
> >> this is the case, I would recommend to use such a Gradle plugin which
> >> automatically sets up the git hooks in the local repository and closes
> >> the gap.
> >>
> >> I will create a Jira issue for this.
> >>
> >> Thanks,
> >>
> >> Michael Brohl
> >>
> >> ecomify GmbH - www.ecomify.de
> >>
> >>
> >> Am 04.02.21 um 11:42 schrieb Girish Vasmatkar:
> >>> Hi Michael/Jacques
> >>>
> >>> +1 for the proposal. However, I do not face this issue on my local,
> >> partly
> >>> because I never tried to push to the repository without running any
> >> gradle
> >>> command. The gitHook gradle plug-in is essentially creating hooks on
> the
> >>> local repository that's why when I push to my forked OFBiz repo,
> pre-push
> >>> hook always gets executed forcing me to conform to checkstyle standards
> >> on
> >>> my forked repo too.
> >>>
> >>> Best Regards,
> >>> Girish
> >>>
> >>> On Thu, Feb 4, 2021 at 1:34 PM Jacques Le Roux <
> >> jacques.le.roux@les7arts.com>
> >>> wrote:
> >>>
> >>>> Le 03/02/2021 à 17:59, Michael Brohl a écrit :
> >>>>> So I think we should find a way to deploy the hooks to the user's
> local
> >>>> repository to make sure they are used there also. Else we would always
> >>>> chase
> >>>>> after newly introduced checkstyle problems, especially if we use pull
> >>>> requests.
> >>>>> I found a solution here:
> >>
> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/
> >>>> (at the bottom of the page). This must be
> >>>>> adapted for Gradle to be dependend of a build.
> >>>>>
> >>>>> This would keep our hooks versioned in the repository and would
> >>>> automatically install the hooks in the local repository.
> >>>>> What do you think?
> >>>> Hi Michael,
> >>>>
> >>>> It looks like a good idea to me. A Jira would fit.
> >>>>
> >>>> In the meantime you need to fix the checkstyle issues put in with
> >>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
> >>>> details here:
> >>>>
> >>>>
> >>
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
> >>>> TIA
> >>>>
> >>>> Jacques
> >>>>
> >>>>
>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Michael Brohl <mi...@ecomify.de>.
Thanks a lot for the comprehensive informations, Aditya!

I somehow missed this activity or simply forgot.

The hooks are not generated by default, during the build, right? If we 
would do this automatically we could close this gap (which is minor 
because the build does the checks anyway).

For plugins, this is indeed a problem because we do not have a build 
mechanism there. We could check through the framework build with 
integrated plugins, which is the only way to automatically test the 
plugins anyway. The problem would be the counter which is targeted at 
the framework code.

Is it possible to specify folder groups or packages to check separately 
so that the framework build could check against one counter for the 
framework code and another counter for the plugins code?

Thanks,

Michael Brohl

ecomify GmbH - www.ecomify.de


Am 04.02.21 um 13:00 schrieb Aditya Sharma:
> Hi Michael,
>
> With OFBIZ-11304[1], we introduced a Gradle plugin
> git-hooks-gradle-plugin[2]. The plugin generates a pre-push hook on the
> developer's local. I think as the plugin persists it should work along with
> the forks too. Along with that, we used GitHub action that runs
> checkstyleMain task on PRs as discussed here[3].
>
> We only introduced these with ofbiz-framework repository and not with
> ofbiz-plugins. We do not have Gradle in place for the ofbiz-plugin
> repository so the same arrangement cannot be done and as discussed here[4]
> it's checked while checking the trunk in Buildbot.
>
> The new checkstyle issues must be introduced from the plugins I guess.
>
>
> 1. https://issues.apache.org/jira/browse/OFBIZ-11304
> 2. https://github.com/jakemarsden/git-hooks-gradle-plugin
> 3.
> https://issues.apache.org/jira/browse/OFBIZ-11304?focusedCommentId=17138097&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17138097
> 4.
> https://issues.apache.org/jira/browse/OFBIZ-11251?focusedCommentId=17183889&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17183889
>
> Thanks and regards,
> Aditya Sharma
>
> On Thu, Feb 4, 2021 at 4:33 PM Michael Brohl <mi...@ecomify.de>
> wrote:
>
>> Hi Girish,
>>
>> which githook plugin are you using? I found several, not sure which one
>> to choose.
>>
>> It seems that they do exactly what I described, just have to check. If
>> this is the case, I would recommend to use such a Gradle plugin which
>> automatically sets up the git hooks in the local repository and closes
>> the gap.
>>
>> I will create a Jira issue for this.
>>
>> Thanks,
>>
>> Michael Brohl
>>
>> ecomify GmbH - www.ecomify.de
>>
>>
>> Am 04.02.21 um 11:42 schrieb Girish Vasmatkar:
>>> Hi Michael/Jacques
>>>
>>> +1 for the proposal. However, I do not face this issue on my local,
>> partly
>>> because I never tried to push to the repository without running any
>> gradle
>>> command. The gitHook gradle plug-in is essentially creating hooks on the
>>> local repository that's why when I push to my forked OFBiz repo, pre-push
>>> hook always gets executed forcing me to conform to checkstyle standards
>> on
>>> my forked repo too.
>>>
>>> Best Regards,
>>> Girish
>>>
>>> On Thu, Feb 4, 2021 at 1:34 PM Jacques Le Roux <
>> jacques.le.roux@les7arts.com>
>>> wrote:
>>>
>>>> Le 03/02/2021 à 17:59, Michael Brohl a écrit :
>>>>> So I think we should find a way to deploy the hooks to the user's local
>>>> repository to make sure they are used there also. Else we would always
>>>> chase
>>>>> after newly introduced checkstyle problems, especially if we use pull
>>>> requests.
>>>>> I found a solution here:
>> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/
>>>> (at the bottom of the page). This must be
>>>>> adapted for Gradle to be dependend of a build.
>>>>>
>>>>> This would keep our hooks versioned in the repository and would
>>>> automatically install the hooks in the local repository.
>>>>> What do you think?
>>>> Hi Michael,
>>>>
>>>> It looks like a good idea to me. A Jira would fit.
>>>>
>>>> In the meantime you need to fix the checkstyle issues put in with
>>>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>>>> details here:
>>>>
>>>>
>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>>>> TIA
>>>>
>>>> Jacques
>>>>
>>>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Aditya Sharma <ad...@apache.org>.
Hi Michael,

With OFBIZ-11304[1], we introduced a Gradle plugin
git-hooks-gradle-plugin[2]. The plugin generates a pre-push hook on the
developer's local. I think as the plugin persists it should work along with
the forks too. Along with that, we used GitHub action that runs
checkstyleMain task on PRs as discussed here[3].

We only introduced these with ofbiz-framework repository and not with
ofbiz-plugins. We do not have Gradle in place for the ofbiz-plugin
repository so the same arrangement cannot be done and as discussed here[4]
it's checked while checking the trunk in Buildbot.

The new checkstyle issues must be introduced from the plugins I guess.


1. https://issues.apache.org/jira/browse/OFBIZ-11304
2. https://github.com/jakemarsden/git-hooks-gradle-plugin
3.
https://issues.apache.org/jira/browse/OFBIZ-11304?focusedCommentId=17138097&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17138097
4.
https://issues.apache.org/jira/browse/OFBIZ-11251?focusedCommentId=17183889&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17183889

Thanks and regards,
Aditya Sharma

On Thu, Feb 4, 2021 at 4:33 PM Michael Brohl <mi...@ecomify.de>
wrote:

> Hi Girish,
>
> which githook plugin are you using? I found several, not sure which one
> to choose.
>
> It seems that they do exactly what I described, just have to check. If
> this is the case, I would recommend to use such a Gradle plugin which
> automatically sets up the git hooks in the local repository and closes
> the gap.
>
> I will create a Jira issue for this.
>
> Thanks,
>
> Michael Brohl
>
> ecomify GmbH - www.ecomify.de
>
>
> Am 04.02.21 um 11:42 schrieb Girish Vasmatkar:
> > Hi Michael/Jacques
> >
> > +1 for the proposal. However, I do not face this issue on my local,
> partly
> > because I never tried to push to the repository without running any
> gradle
> > command. The gitHook gradle plug-in is essentially creating hooks on the
> > local repository that's why when I push to my forked OFBiz repo, pre-push
> > hook always gets executed forcing me to conform to checkstyle standards
> on
> > my forked repo too.
> >
> > Best Regards,
> > Girish
> >
> > On Thu, Feb 4, 2021 at 1:34 PM Jacques Le Roux <
> jacques.le.roux@les7arts.com>
> > wrote:
> >
> >> Le 03/02/2021 à 17:59, Michael Brohl a écrit :
> >>> So I think we should find a way to deploy the hooks to the user's local
> >> repository to make sure they are used there also. Else we would always
> >> chase
> >>> after newly introduced checkstyle problems, especially if we use pull
> >> requests.
> >>> I found a solution here:
> >>
> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/
> >> (at the bottom of the page). This must be
> >>> adapted for Gradle to be dependend of a build.
> >>>
> >>> This would keep our hooks versioned in the repository and would
> >> automatically install the hooks in the local repository.
> >>> What do you think?
> >> Hi Michael,
> >>
> >> It looks like a good idea to me. A Jira would fit.
> >>
> >> In the meantime you need to fix the checkstyle issues put in with
> >> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
> >> details here:
> >>
> >>
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
> >>
> >> TIA
> >>
> >> Jacques
> >>
> >>
>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Michael Brohl <mi...@ecomify.de>.
Hi Girish,

which githook plugin are you using? I found several, not sure which one 
to choose.

It seems that they do exactly what I described, just have to check. If 
this is the case, I would recommend to use such a Gradle plugin which 
automatically sets up the git hooks in the local repository and closes 
the gap.

I will create a Jira issue for this.

Thanks,

Michael Brohl

ecomify GmbH - www.ecomify.de


Am 04.02.21 um 11:42 schrieb Girish Vasmatkar:
> Hi Michael/Jacques
>
> +1 for the proposal. However, I do not face this issue on my local, partly
> because I never tried to push to the repository without running any gradle
> command. The gitHook gradle plug-in is essentially creating hooks on the
> local repository that's why when I push to my forked OFBiz repo, pre-push
> hook always gets executed forcing me to conform to checkstyle standards on
> my forked repo too.
>
> Best Regards,
> Girish
>
> On Thu, Feb 4, 2021 at 1:34 PM Jacques Le Roux <ja...@les7arts.com>
> wrote:
>
>> Le 03/02/2021 à 17:59, Michael Brohl a écrit :
>>> So I think we should find a way to deploy the hooks to the user's local
>> repository to make sure they are used there also. Else we would always
>> chase
>>> after newly introduced checkstyle problems, especially if we use pull
>> requests.
>>> I found a solution here:
>> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/
>> (at the bottom of the page). This must be
>>> adapted for Gradle to be dependend of a build.
>>>
>>> This would keep our hooks versioned in the repository and would
>> automatically install the hooks in the local repository.
>>> What do you think?
>> Hi Michael,
>>
>> It looks like a good idea to me. A Jira would fit.
>>
>> In the meantime you need to fix the checkstyle issues put in with
>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
>> details here:
>>
>> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>>
>> TIA
>>
>> Jacques
>>
>>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Girish Vasmatkar <gi...@hotwaxsystems.com>.
Hi Michael/Jacques

+1 for the proposal. However, I do not face this issue on my local, partly
because I never tried to push to the repository without running any gradle
command. The gitHook gradle plug-in is essentially creating hooks on the
local repository that's why when I push to my forked OFBiz repo, pre-push
hook always gets executed forcing me to conform to checkstyle standards on
my forked repo too.

Best Regards,
Girish

On Thu, Feb 4, 2021 at 1:34 PM Jacques Le Roux <ja...@les7arts.com>
wrote:

> Le 03/02/2021 à 17:59, Michael Brohl a écrit :
> > So I think we should find a way to deploy the hooks to the user's local
> repository to make sure they are used there also. Else we would always
> chase
> > after newly introduced checkstyle problems, especially if we use pull
> requests.
> >
> > I found a solution here:
> https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/
> (at the bottom of the page). This must be
> > adapted for Gradle to be dependend of a build.
> >
> > This would keep our hooks versioned in the repository and would
> automatically install the hooks in the local repository.
> >
> > What do you think?
>
> Hi Michael,
>
> It looks like a good idea to me. A Jira would fit.
>
> In the meantime you need to fix the checkstyle issues put in with
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
> details here:
>
> https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio
>
> TIA
>
> Jacques
>
>

Re: Checkstyle pre-push hook not used when merging PR's

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 03/02/2021 à 17:59, Michael Brohl a écrit :
> So I think we should find a way to deploy the hooks to the user's local repository to make sure they are used there also. Else we would always chase 
> after newly introduced checkstyle problems, especially if we use pull requests.
>
> I found a solution here: https://www.viget.com/articles/two-ways-to-share-git-hooks-with-your-team/ (at the bottom of the page). This must be 
> adapted for Gradle to be dependend of a build.
>
> This would keep our hooks versioned in the repository and would automatically install the hooks in the local repository.
>
> What do you think? 

Hi Michael,

It looks like a good idea to me. A Jira would fit.

In the meantime you need to fix the checkstyle issues put in with
https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973
details here:
https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1973/steps/check/logs/stdio

TIA

Jacques