-
Notifications
You must be signed in to change notification settings - Fork 488
Build using openjdk8 #550
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
Build using openjdk8 #550
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.
Thanks for the PR. I left comments inline
@@ -2,7 +2,7 @@ | |||
task downloadJavacc { | |||
ext.file = "javacc-5.0.tar.gz" | |||
ext.dest = file("$buildDir/javacc/$file") | |||
ext.url = "https://javacc.org/downloads/$file" | |||
ext.url = "http://download.openpkg.org/components/cache/javacc/$file" |
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.
Is there a reason that this was updated for using openjdk8? If we do this we'd like this to be a URL that works over https
.
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.
No reason with openjdk8.
It is another issue with the gradle build (last successful build was 1 year ago).
I didn't find an URL over https
which serve javacc-5.0.tar.gz
(i don"t know why this version does not exist on the new javacc download page https://github.com/javacc/javacc/releases)
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.
Let's split this up into another PR. The openjdk8 is a no brainer. It appears due to this not being available over https, javacc may take a bit more work.
distributionUrl=https\://services.gradle.org/distributions/gradle-3.1-all.zip |
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.
Good catch here. I'd prefer things to be distinct commits so I went ahead and pushed a separate commit to fix this.
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.
OK
Thanks for the Pull Request! This is now merged into master via 50dae66 😄 |
see comment