[jira] [Commented] (FINERACT-1001) Enable and Enforce PMD on Fineract

["Michael Vorburger (Jira)" <ji...@apache.org>]

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

Michael Vorburger commented on FINERACT-1001:
---------------------------------------------

[~awasum] nope, because I have not used PMD myself in projects in my previous lifes... :P but I have used Checkstyle and SpotBugs and Error Prone. I'm personally not entirely sure how much additional value PMD adds to the project, now that we have made great progress with CS, SB and EP. 

Perhaps what could be interesting would be to have a closer look at your attached main.html report, and over https://pmd.github.io/pmd-6.24.0/pmd_rules_java.html ... I suspect completely addressing each and every one of all 19575 rule violations is not going to be possible in a reasonable amount of time - but perhaps there are a few "high value" ones we can pick? Or decide it's "not worth it at all". I only had a very quick look over the main.html, and didn't really see anything there *yet* that made me think like "oh wow yes that sounds like a real bug, glad it caught that". One that I liked, which I don't think CS/SB/EP caught, is "Using equalsIgnoreCase() is cleaner than using toUpperCase/toLowerCase().equals()."... that's a good one. But that alone, if we don't find other good catches, perhaps doesn't justify the introduction of a 4th code quality control tool?

Most other ones seems to be much more "style" than "bug" (or raises things which I suspect our recent CS/SB/EP work meanwhile already addressed). But it's kind of hard to read, because it's "by occurrence" instead of "by kind of problem" - if it's possible to customize the reporting, perhaps that would make it easier to analyze and discuss what (if anything at all...) could be "worth it".

One thing I'm curious about is how much extra build time overhead adding the latest version of PMD adds, now that we have the latest Gradle. It's something to keep in mind, because I remember having raised https://github.com/apache/fineract/pull/612 because it was really slowing the build down. [~Percy Ashu] I'm curious if re-adding PMD now adds just like an extra 10% or xN like 300% or so to a "./gradle clean build" - do you want to try?

Something else I feel quite strongly about is that just having a PR which re-adds PMD and prints a lot of warnings (as it originally used to) doesn't really help the project. Only a more fine grained approach of suppressing warnings, and gradually re-enabling any "high-value" ones, step by step in PRs which actually fix violations, and then enforce those (by failing the build if there are regression), like we did for CS, SB and EP, really makes sense. I don't even know if that's possible with PMD configuration.. if not, perhaps we shouldn't adopt it. Let's find out!

> Enable and Enforce PMD on Fineract
> ----------------------------------
>
>                 Key: FINERACT-1001
>                 URL: https://issues.apache.org/jira/browse/FINERACT-1001
>             Project: Apache Fineract
>          Issue Type: Sub-task
>            Reporter: Awasum Yannick
>            Priority: Major
>             Fix For: 1.5.0
>
>         Attachments: main.html
>
>
> Enable and Enforce PMD ( [https://pmd.github.io/ )|https://pmd.github.io/] on Fineract. Make PMD fail until violations are fixed just like we do for Spotbug, checkstyle etc. See previous work here:
> [https://github.com/apache/fineract/pull/295]
> See where PMD was commented out because it was not use: [https://github.com/apache/fineract/pull/612]



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