[jira] [Comment Edited] (OFBIZ-11304) Install a Checkstyle pre-push (on every committer machine?)

["Aditya Sharma (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OFBIZ-11304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17116441#comment-17116441 ] 

Aditya Sharma edited comment on OFBIZ-11304 at 5/26/20, 5:29 AM:
-----------------------------------------------------------------

Reference to recent discussion on dev mailing list:

[https://markmail.org/message/4x6tt4qebx4hqv7x]

Quoting my comment:
{quote}I think *check styles* plugin is important for us and we should think of introducing a pre-commit hook that checks only for the staged changes. This will help developers to know about the lint issues before the commit itself.

Some reference I found that achieves the above case. Though I didn't try any of these solutions.

[https://ebaytech.berlin/checkstyle-on-changed-files-with-gradle-1619e49dbe4e] [https://stackoverflow.com/questions/43730901/is-there-a-way-to-run-checkstyle-on-only-files-that-have-changes-in-vcs] [https://ealebed.github.io/posts/2020/gradle-checkstyle-on-changed-files-only/]
{quote}
 

Adding to that

 I think we can also use a Gradle plugin to add pre-commit hook. This way it can be more configurable for the OFBiz community to make appropriate changes to it when needed.

Here is the link to PR I have created
 [https://github.com/apache/ofbiz-framework/pull/161/files]

In this PR, check style works only on staged files.

To test the above PR:
 1. Create a test branch on local
 2. Add some spacing or any other change in java file that doesn't comply to standards
 3. Try a test commit

I can think of only one issue with above code that if the file staged already has some lint issues it will show errors. We can also fix them with the current work mentioning it commit, this will reduce the existing burden. Though we can always use no-verify flag to bypass the pre-commit hook.
{code:java}
git commit --no-verify
 
{code}
{{Wdyat?}}


was (Author: aditya.sharma):
Reference to recent discussion on dev mailing list:

https://markmail.org/message/4x6tt4qebx4hqv7x

Quoting my comment:
{quote}I think *check styles* plugin is important for us and we should think of introducing a pre-commit hook that checks only for the staged changes. This will help developers to know about the lint issues before the commit itself.

Some reference I found that achieves the above case. Though I didn't try any of these solutions.

[https://ebaytech.berlin/checkstyle-on-changed-files-with-gradle-1619e49dbe4e] [https://stackoverflow.com/questions/43730901/is-there-a-way-to-run-checkstyle-on-only-files-that-have-changes-in-vcs] [https://ealebed.github.io/posts/2020/gradle-checkstyle-on-changed-files-only/]
{quote}
 

Adding to that

 I think we can also use a Gradle plugin to add pre-commit hook. This way it can be more configurable for the OFBiz community to make appropriate changes to it when needed.

Here is the link to PR I have created
[https://github.com/apache/ofbiz-framework/pull/161/files]

In this PR, check style works only on staged files.

To test the above PR:
1. Create a test branch on local
2. Add some spacing or any other change in java file that doesn't comply to standards
3. Try a test commit 

I can think of only one issue with above code that if the file staged already has some lint issues it will show errors. We can also fix them with the current work mentioning it commit, this will reduce the existing burden. Though we can always use no-verify flag to bypass the pre-commit hook.

{{}}
{code:java}
git commit --no-verify
 
{code}
{{Wdyat?}}

> Install a Checkstyle pre-push (on every committer machine?)
> -----------------------------------------------------------
>
>                 Key: OFBIZ-11304
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-11304
>             Project: OFBiz
>          Issue Type: Sub-task
>    Affects Versions: Trunk
>            Reporter: Jacques Le Roux
>            Priority: Minor
>             Fix For: Upcoming Branch
>
>
> The ofbizTrunkFrameworkPlugins build fails when a lint error is detected by the check gradle task. It's "hard" to exactly know from where lint errors  come among all still present.
> I think we should rely on a Checkstyle pre-commit hook like https://gist.github.com/davetron5000/37350 to complement tasks.checkstyleMain.maxErrors. This pre-commit hook prevents to commit when a lint error is present in the commit. 
> Every committer would have it installed locally and the problem would be gone with some committers good will. I started a discussion about it at https://markmail.org/message/guxbsvdkzky7gtdx. Jacopo made the same proposition years ago: https://markmail.org/message/gkgmko4axj3vtnv3



--
This message was sent by Atlassian Jira
(v8.3.4#803005)