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

Java10 Hacking #113

Closed
wants to merge 7 commits into from
Closed

Conversation

svanoort
Copy link
Member

@svanoort svanoort commented Jun 19, 2018

  • Handle Java version strings like "9" and "10" due to change in the nomenclature from 1.8, etc
    • Note that this requires all things taking up new plugin POM to change their java.level, i.e. "8" -> "1.8"

TODO:

  • org.codehaus.mojo.signature artifactid hardcoded, because it does NOT include a period, i.e. 1.8 -> 18

pom.xml Outdated
@@ -521,7 +521,7 @@
<exclude>org.kohsuke.stapler:stapler-jelly</exclude>
<exclude>org.kohsuke.stapler:stapler-jrebel</exclude>
<exclude>org.jenkins-ci:task-reactor</exclude>
<!-- findbugs dep managed to provided and optional so is not shipped and missing annotations ok -->
<!-- findbugs dep managed to provided and optional so is not shipped and missing 3ations ok -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

@KostyaSha
Copy link
Member

will guice/guava/sezpoz as usual break? :D

@svanoort
Copy link
Member Author

will guice/guava/sezpoz as usual break? :D

Runs fine with Java 10 as of the latest builds AFAICT.

The problem is all the tooling and so forth.

@oleg-nenashev
Copy link
Member

@jenkinsci/java10-support

@KostyaSha
Copy link
Member

The problem is all the tooling and so forth.

Where is my popcorn! :)

@oleg-nenashev
Copy link
Member

@svanoort maybe it makes sense to integrate with master (#114) and bump Animal Sniffer to 1.17

@svanoort
Copy link
Member Author

@oleg-nenashev Done -- now to figure out how we deal with the org.codehaus.mojo.signature dependencies?

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose a more universal approach.

  • Introduce java.effective.level property which defaults to 1.${java.level}
  • Introduce java.effective.level.test property which defaults to java.effective.level
  • Introduce codehaus.mojo.signature.artifactId prioperty which defaults to java1${java.level}
  • Use new properties in the subject code
  • Introduce JDK10 and JDK11 profiles which activate on proper java.level values and override the variables

It would allow to deliver this change without breaking compatibility with existing Plugin POM and without hardcoding anything.

WDYT @svanoort ?

@svanoort
Copy link
Member Author

@oleg-nenashev I agree those are better approaches -- I hadn't considered that approach, but it seems a lot better.

@svanoort svanoort closed this Sep 22, 2018
@svanoort svanoort reopened this Sep 22, 2018
@@ -628,7 +628,8 @@
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java1${java.level}</artifactId>
<!-- Hardcoded unfortunately -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear comment

@batmat
Copy link
Member

batmat commented Jan 9, 2019

This is now conflicted, and I suspect superseded.

@svanoort I'm planning to close this in the next days if you don't object. And obviously, if you come after I closed it, feel free to reopen if you think this must still get in.

Thanks!

@batmat
Copy link
Member

batmat commented Jan 22, 2019

Closing as per my last comment.

@batmat batmat closed this Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants