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

Set Java version for SonarQube to disable checks only relevant for ne… #829

merged 1 commit into from Nov 14, 2015


Copy link

@hansjoachim hansjoachim commented Nov 14, 2015

…wer Java versions, e.g. diamond operators ("<>")

The SonarQube instance which scans the FitNesse code from time to time has various rules to take advantage of Java 7 and 8 features/functionality. For examples, see java7 and java8 in the issue tag cloud on the dashboard Since FitNesse targets Java 6 these don't help that much though.

Therefore I've added the Java version to SonarQube properties so that checking for these kind of issues will be skipped. It's untested since my local SonarQube instance didn't seem to have the diamond operator issue at least, but I believe it should be easy to see once the next scan is performed.

…wer Java versions, e.g. diamond operators ("<>")
Copy link

@amolenaar amolenaar commented Nov 14, 2015

We'll just add it and see what happens :).

But now that you mention it. Shouldn't we set the Java version to Java 7 or 8? I think the advantages of Java 7 are minor, but Java 8 adds some nice features, like default implementations in interfaces, closures and a stream() api on collections.

@amolenaar amolenaar merged commit 3c3c138 into unclebob:master Nov 14, 2015
amolenaar added a commit that referenced this issue Nov 14, 2015
Set Java version for SonarQube to disable checks only relevant for ne…
@amolenaar amolenaar added this to the Next release milestone Nov 14, 2015
Copy link
Contributor Author

@hansjoachim hansjoachim commented Nov 21, 2015

Looks like it worked, because the amount of open issues has dropped now. Of course, I now notice I forgot to include the documentation link which actually explained the option I added

But now that you mention it. Shouldn't we set the Java version to Java 7 or 8?

For building FitNesse in general? I just picked it based on the source and target version set in build.xml.

I don't know, and I am not sure how conservative FitNesse has been in the past with Java version requirements. I guess it depends on the wider community/use of FitNesse. I would assume it is pretty common to include FitNesse as a part of the build of a project to be able to run the tests at build time, and not not all of these projects/build pipelines will have updated to the latest JDK. Then again, I don't know how likely it is that these kind of projects will be upgrading dependencies.

For comparison with other projects, if you look at something like Jenkins, they've only recently started requiring Java 7 in order to run (see [changelog entry for 1.625.1|]). I get the overall impression that frameworks/libraries tries to support the currently used JDKs and only upgrade requirements once it is safe to assume that the vast majority who can and will upgrade have done so. I think some libraries have started to require Java 7 now, but I can't remember seeing anyone require Java 8, apart from smaller add-on libraries or extensions which add certain Java 8 APIs for those who can use them.

It's up to you I guess, but I don't think I'd jump straight to Java8, but rather do it incrementally. Maybe once the next release is out, add a note to the release notes warning that the release after that one will most likely require Java 7. Might get some feedback and discussion out of that.

As a side note, the JDK will become more aggressive and remove source and target options earlier (see, though they will still keep the current + three releases back, so even JDK9 will support 1.6.

@hansjoachim hansjoachim deleted the SonarQube-javaversion branch Nov 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants