-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add support for Java 11 #220
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good in general. Just a couple minor scoping issues... :-)
.travis.yml
Outdated
@@ -1,9 +1,10 @@ | |||
sudo: false | |||
language: java | |||
jdk: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the jdk
tag, maybe consider a build matrix like:
matrix:
include:
- jdk: openjdk8
- jdk: oraclejdk8
- jdk: openjdk9
- jdk: oraclejdk9
- jdk: openjdk10
- jdk: oraclejdk10
- jdk: openjdk11
- os: osx
osx_image: xcode10
droid-binary/bin/droid.sh
Outdated
@@ -146,7 +146,7 @@ fi | |||
|
|||
# Run the command-line or user interface version with the options: | |||
if [ $# -gt 0 ]; then | |||
java -XX:+IgnoreUnrecognizedVMOptions --add-modules=java.xml.bind,java.xml.ws,java.xml.ws.annotation $OPTIONS -jar "$DROID_HOME/droid-command-line-${project.version}.jar" "$@" | |||
java -XX:+IgnoreUnrecognizedVMOptions $OPTIONS -jar "$DROID_HOME/droid-command-line-${project.version}.jar" "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-XX:+IgnoreUnrecognizedVMOptions
is only needed to allow --add-modules
on JDKs prior to 9.
As you have removed the --add-modules
argument you can also drop the -XX:+IgnoreUnrecognizedVMOptions
argument.
droid-binary/bin/droid.sh
Outdated
else | ||
java -XX:+IgnoreUnrecognizedVMOptions --add-modules=java.xml.bind,java.xml.ws,java.xml.ws.annotation $OPTIONS -jar "$DROID_HOME/droid-ui-${project.version}.jar" | ||
java -XX:+IgnoreUnrecognizedVMOptions $OPTIONS -jar "$DROID_HOME/droid-ui-${project.version}.jar" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
</dependency> | ||
<dependency> | ||
<groupId>org.glassfish.jaxb</groupId> | ||
<artifactId>jaxb-runtime</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
droid-parent/pom.xml
Outdated
<hibernate.version>5.4.1.Final</hibernate.version> | ||
<derby.version>10.13.1.1</derby.version> | ||
<cxf.version>3.2.0</cxf.version> | ||
<aspectj.version>1.8.11</aspectj.version> | ||
<aspectj.version>1.8.13</aspectj.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably go to 1.9.1
- which is compatible with JDK 8.
</dependency> | ||
<dependency> | ||
<groupId>com.sun.activation</groupId> | ||
<artifactId>javax.activation</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
<dependency> | ||
<groupId>com.sun.activation</groupId> | ||
<artifactId>javax.activation</artifactId> | ||
<version>1.2.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
</dependency> | ||
<dependency> | ||
<groupId>com.sun.activation</groupId> | ||
<artifactId>javax.activation</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
</dependency> | ||
<dependency> | ||
<groupId>org.glassfish.jaxb</groupId> | ||
<artifactId>jaxb-runtime</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
</dependency> | ||
<dependency> | ||
<groupId>com.sun.activation</groupId> | ||
<artifactId>javax.activation</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably have <scope>runtime</scope>
?
- jdk: openjdk11 | ||
- os: osx | ||
osx_image: xcode10 | ||
before_script: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add an empty before_script
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Otherwise it use default before_script
which is primary for linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok.
|
||
import uk.gov.nationalarchives.droid.core.interfaces.config.RuntimeConfig; | ||
|
||
/** | ||
* @author rflitcroft | ||
* | ||
*/ | ||
@Ignore("Disable for instability") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's that all about then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's test which measure time for throttling. But sometimes when travis-ci is overloaded, this test break whole build.
No description provided.