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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

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

Nicolas Malin (Jira)

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

Aditya Sharma edited comment on OFBIZ-11304 at 5/27/20, 3:11 PM:
-----------------------------------------------------------------

>> is there a way to block a commit if tasks.checkstyleMain.maxErrors increases?

I think that is doable if we replace our custom task checkstyleChanged with existing checkstyleMain. 

[https://github.com/apache/ofbiz-framework/pull/161/files#diff-c197962302397baf3a4cc36463dce5eaR297]

Like this
{code:java}
hooks = ['pre-commit': 'checkstyleMain']{code}
 

That way we will not need any custom task, just this plugin to add a pre-commit hook.  I tried it locally and it gives an error such that:

 
{code:java}
Configure project :
You are using one or more deprecated Asciidoctor Gradle plugin features. To help with migration run with --warning-mode=all.
> Task :compileJava UP-TO-DATE
> Task :compileGroovy UP-TO-DATE
> Task :processResources UP-TO-DATE
> Task :classes UP-TO-DATE
> Task :checkstyleMain FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/aditya/sandbox/apache/ofbiz-framework/build/reports/checkstyle/main.html
 Checkstyle files with violations: 1019
 Checkstyle violations by severity: [error:27595]
{code}
 

That sounds promising. This will not give the exact reason but alert the developer that something needs to be taken care of.
 If this looks good I will remove the custom task and apply these changes. Also, I  will explore more if we could show the exact error.


was (Author: aditya.sharma):
>> is there a way to block a commit if tasks.checkstyleMain.maxErrors increases?

I think that is doable if we replace our custom task checkstyleChanged with existing checkstyleMain. 

[https://github.com/apache/ofbiz-framework/pull/161/files#diff-c197962302397baf3a4cc36463dce5eaR297]

That way we will not need any custom task, just this plugin to add a pre-commit hook.  I tried it locally and it gives an error such that:

 
{code:java}
Configure project :
You are using one or more deprecated Asciidoctor Gradle plugin features. To help with migration run with --warning-mode=all.
> Task :compileJava UP-TO-DATE
> Task :compileGroovy UP-TO-DATE
> Task :processResources UP-TO-DATE
> Task :classes UP-TO-DATE
> Task :checkstyleMain FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/aditya/sandbox/apache/ofbiz-framework/build/reports/checkstyle/main.html
 Checkstyle files with violations: 1019
 Checkstyle violations by severity: [error:27595]
{code}
 

That sounds promising. This will not give the exact reason but alert the developer that something needs to be taken care of.
If this looks good I will remove the custom task and apply these changes. Also, I  will explore more if we could show the exact error.

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