jenkinsci / rubymetrics-plugin

Ruby metric reports for Jenkins. Rcov, Rails stats, Rails notes and Flog.
https://plugins.jenkins.io/rubyMetrics/
25 stars 28 forks source link

Fix issues reported by FindBugs #30

Open pkuczynski opened 7 years ago

pkuczynski commented 7 years ago

After upgrading to newer parent pom as suggested in the troubleshooting section of Hosting Plugins on Jenkins wiki: https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-Workingaroundcommonissues

<parent>
    <groupId>org.jenkins-ci.plugins</groupId>
    <artifactId>plugin</artifactId>
    <version>2.11</version>
    <relativePath/>
  </parent>

Over 30 issues are being reported by FindBugs which fails the build. This should be fixed before any other work continues...

For the time being I disabled FindBugs in pom.xml:

<plugin>
    <groupId>org.codehaus.mojo</groupId>
    <artifactId>findbugs-maven-plugin</artifactId>
    <configuration>
        <skip>true</skip>
    </configuration>
</plugin>

Any volunteers willing to help fixing those issues?

md5 commented 7 years ago

@pkuczynski I've fixed about half of the warnings in my findbugs-fixes branch: https://github.com/jenkinsci/rubymetrics-plugin/compare/6ca45e100d9018ad512876e3393104e240d34959...2fb3781a1cdeed2232d50f2e884bc9aa916a6bb2

The remaining warnings involve the following:

md5 commented 7 years ago

One thing that's probably worth mentioning is that I removed a number of method of the form: public T parse(InputStream input) throws IOException

I doubt that anyone is relying on these methods being public, but we could probably add them back.

pkuczynski commented 7 years ago

@md5 thanks for your efforts! When it comes to the remaining issues, I have no clue how to fix them and I will happily follow your suggestions...

md5 commented 7 years ago

@pkuczynski I think assuming UTF-8 and removing the Serializable from the file detail classes is probably safe, but I need to do a bit more research. Not sure when I'll get the time.

As for the stuff in RcovAbstractResult, I think it would be best to have it internally store numeric values and to format them on display, but the thing I'm worried about is whether these objects are serialized as part of completed builds. If so, then simply changing the fields is not an option since we would need to handle old builds that were persisted with the String values. I suspect these objects are part of the persisted build, but I haven't looked closely enough to say for sure.