Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues reported by FindBugs #30

Open
pkuczynski opened this issue Sep 29, 2016 · 4 comments
Open

Fix issues reported by FindBugs #30

pkuczynski opened this issue Sep 29, 2016 · 4 comments

Comments

@pkuczynski
Copy link
Contributor

pkuczynski commented Sep 29, 2016

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?

pkuczynski added a commit that referenced this issue Sep 29, 2016
See the sample POM for new Jenkins plugins: https://github.com/jenkinsci/maven-hpi-plugin/blob/master/hpi-archetype/pom.xml

Skipped upgrading parent pom, as it introduce mandatory FindBugs execution, which fails during the build. I will create a separate issue to fix those #30.

See Hosting Plugins on Jenkins wiki: https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-Workingaroundcommonissues
pkuczynski added a commit that referenced this issue Sep 29, 2016
See the sample POM for new Jenkins plugins: https://github.com/jenkinsci/maven-hpi-plugin/blob/master/hpi-archetype/pom.xml

Skipped upgrading parent pom, as it introduce mandatory FindBugs execution, which fails during the build. I will create a separate issue to fix those #30.

See Hosting Plugins on Jenkins wiki: https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-Workingaroundcommonissues
pkuczynski added a commit that referenced this issue Sep 29, 2016
See the sample POM for new Jenkins plugins: https://github.com/jenkinsci/maven-hpi-plugin/blob/master/hpi-archetype/pom.xml

Temporarily disable FindBugs execution as it fails during the build. Separate issue created to fix those problems #30.

See Hosting Plugins on Jenkins wiki: https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-Workingaroundcommonissues
pkuczynski added a commit that referenced this issue Sep 29, 2016
See the sample POM for new Jenkins plugins: https://github.com/jenkinsci/maven-hpi-plugin/blob/master/hpi-archetype/pom.xml

Temporarily disable FindBugs execution as it fails during the build. Separate issue created to fix those problems #30.

See Hosting Plugins on Jenkins wiki: https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-Workingaroundcommonissues
@md5
Copy link
Contributor

md5 commented Sep 30, 2016

@pkuczynski I've fixed about half of the warnings in my findbugs-fixes branch: 6ca45e1...2fb3781

The remaining warnings involve the following:

  • DM_DEFAULT_ENCODING: There a number of places that are receiving byte-level output from rake commands that aren't parsing them with a character set. It's unclear which character set should be used here. I may be safe to just assume they're UTF-8
  • BX_UNBOXING_IMMEDIATELY_REBOXED, DM_BOXED_PRIMITIVE_FOR_PARSING: The RcovAbstractResult is storing a number of metrics as strings and then repeatedly parsing and unparsing them to calculate derived Integer and Float values
  • SE_BAD_FIELD: RcovFileDetail and SaikuroFileDetail are marked as Serializable, but they have non-Serializable fields. It seems to me that these classes should not be Serializable since they are only instantiated on the Jenkins master from build actions and don't need to travel across the wire. There are also no fields of these types

@md5
Copy link
Contributor

md5 commented Sep 30, 2016

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
Copy link
Contributor Author

@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...

  • DM_DEFAULT_ENCODING - I guess we can assume UTF-8
  • BX_UNBOXING_IMMEDIATELY_REBOXED - shall we convert the values on the RcovAbstractResult level? Or what are the other options?
  • SE_BAD_FIELD - I guess we can make them no Serializable then...

@md5
Copy link
Contributor

md5 commented Sep 30, 2016

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants