[MCOMPILER-534] Document conditional setting of the --release property

[aherbert]

Tested using:

mvn site:site -DgenerateReports=false

[garydgregory]

@aherbert ping, the build failed. You might have to rerun the job if the logs have already been removed.

[aherbert]

The build fails due to something totally unrelated. This is a change to the site documentation. That is not what is failing the build. When I looked at this the other PRs, and also the master branch, were also failing. I will rebase and push to collect a fix that I presume is now in master.

[michael-o]

JDK 9 or later

[aherbert]

The whole point is you want to be able to build with JDK 8 or later. If you want to only build with JDK 9 or later then you do not have to use conditional setting of the --release option.

[michael-o]

That is not what the sentence tells me. It says: Targettting Java 8 need JDK 8 or later and it requires this flag. What am I misunderstanding? I understand what you say and you are right, but the documentation is not for me.

[aherbert]

Change "it requires this flag" to "it requires this flag when using a JDK later than 8".

If you use the flag with JDK 8 then the build breaks. This was the original source of [MCOMPILER-534]. You can only use the flag with JDK 9+:

Target   Tool chain   --release flag    Result
Java 8   JDK 8        Y                 build error
Java 8   JDK 8        N                 OK
Java 8   JDK 9        Y                 OK (compiler targets Java 8)
Java 8   JDK 9        N                 compiler targets Java 9 API methods that supersede Java 8 methods

If this is not the desired behaviour, i.e. the --release flag should be harmless if set on JDK 8, then something should be changed in the compiler plugin. If you should not use the flag when using JDK 8, then this is what I am trying to address with this documentation.

[michael-o]

The table does not look right, line 2. No JDK 8 exhibits --release. You can never provide it on JDK 8. What am I missing again?

[aherbert]

The table --release column should really read set <maven.compiler.release>8</maven.compiler.release> but that was too long. Forgive my shorthand with no explanation.

When you set <maven.compiler.release>8</maven.compiler.release> then the maven plugin will use the --release flag. This is not ignored by JDK 8; as you state it does not exhibit this flag as an option. Using it raises an error. So you cannot set it when building with JDK 8. You should only set it for JDK 9+.

If this is not clear from my documentation then we can revise it. It is trying to state:

1. Do not use this flag when building with JDK 8
2. Use it when building with a later JDK (i.e. 9+) and targeting Java 8

I think a perfectly valid example is a project with a toolchain that uses JDK 8 only (i.e. it can be built with JDK 8). This can still be compiled with JDK 11. But that may end up targeting the wrong Java API if some methods/classes have had updates. So your project then should conditionally set the <maven.compiler.release> property. If you always set it then you have problems on JDK 8.

[michael-o]

Please rework the docs. I prefer rather no docs than confusing docs.

[aherbert]

Revised docs to explicitly state building "using JDK 8 and also JDK 9 or later". I think this is crux of the issue. If you only use one toolchain for the build then there is no issue, just configure it for that toolchain. If you wish your build to be flexible then you require conditional setting of the property.

[michael-o]

This looks good to me, but the issue type is bug and not improvement, plus the summary does not match. Should I update to match with your PR since we aren't going to change the code and there is no bug here, but so by design. WDYT?

[aherbert]

Sure, update the Jira ticket as you see fit. This is more documentation of the current intent of the plugin.

I am not familiar with the CI workflow but something does not seem correct with the current build; I believe it to be unrelated to this doc change.

[michael-o]

Sure, update the Jira ticket as you see fit. This is more documentation of the current intent of the plugin.

I am not familiar with the CI workflow but something does not seem correct with the current build; I believe it to be unrelated to this doc change.

Will address this PR tomorrow.