You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/09/28 04:49:22 UTC
[GitHub] [thrift] Jimexist opened a new pull request, #2686: java to use jdk 17
Jimexist opened a new pull request, #2686:
URL: https://github.com/apache/thrift/pull/2686
<!-- Explain the changes in the pull request below: -->
java to use jdk 17, this is a second try after #2662
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [ ] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? (not required for trivial changes)
- [ ] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [ ] Did you squash your changes to a single commit? (not required, but preferred)
- [ ] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2686: THRIFT-5644: java to use jdk 17
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2686:
URL: https://github.com/apache/thrift/pull/2686#issuecomment-1260868863
even if the cross tests might see some errors given they are not related to this fix i'll continue to merge this as a fix because it has resolved the long standing Java TLS error
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2686: THRIFT-5644: java to use jdk 17 and upgrade minimal language level to 11
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2686:
URL: https://github.com/apache/thrift/pull/2686#issuecomment-1274025926
@fishy @ctubbsii i have updated the minimal JDK requirement to be 11, please take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ctubbsii commented on a diff in pull request #2686: THRIFT-5644: java to use jdk 17
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2686:
URL: https://github.com/apache/thrift/pull/2686#discussion_r983733172
##########
lib/java/gradle/sourceConfiguration.gradle:
##########
@@ -53,13 +53,6 @@ tasks.withType(JavaCompile).configureEach {
]
}
-tasks.withType(Javadoc) {
- failOnError false
- options.addStringOption('Xdoclint:none', '-quiet')
- options.addStringOption('encoding', 'UTF-8')
- options.addStringOption('charSet', 'UTF-8')
-}
-
Review Comment:
Why not keep this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a diff in pull request #2686: THRIFT-5644: java to use jdk 17 and upgrade minimal language level to 11
Posted by GitBox <gi...@apache.org>.
fishy commented on code in PR #2686:
URL: https://github.com/apache/thrift/pull/2686#discussion_r995193436
##########
build/docker/README.md:
##########
@@ -180,7 +180,7 @@ Last updated: October 1, 2017
| erlang | OTP-18 | OTP-23 | |
| go | 1.15.10 | 1.16.2 | |
| haxe | 3.2.1 | 3.4.4 | THRIFT-4352: avoid 3.4.2 |
-| java | 1.8.0\_191 | 11.0.3 | |
+| java | 1.8.0\_191 | 17 | |
Review Comment:
we should also update the min version here? I think we dropped support to jdk8 with this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2686: THRIFT-5644: java to use jdk 17
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2686:
URL: https://github.com/apache/thrift/pull/2686#issuecomment-1262945933
turns out that upgrading to java 17 won't solve the TSL runtime error during cross test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on a diff in pull request #2686: THRIFT-5644: java to use jdk 17 and upgrade minimal language level to 11
Posted by GitBox <gi...@apache.org>.
Jimexist commented on code in PR #2686:
URL: https://github.com/apache/thrift/pull/2686#discussion_r989876500
##########
lib/java/gradle/sourceConfiguration.gradle:
##########
@@ -53,13 +53,6 @@ tasks.withType(JavaCompile).configureEach {
]
}
-tasks.withType(Javadoc) {
- failOnError false
- options.addStringOption('Xdoclint:none', '-quiet')
- options.addStringOption('encoding', 'UTF-8')
- options.addStringOption('charSet', 'UTF-8')
-}
-
Review Comment:
reverted
##########
lib/java/Makefile.am:
##########
@@ -23,7 +23,8 @@ all-local:
$(GRADLE) $(GRADLE_OPTS) assemble \
-Prelease=true \
-Pthrift.version=$(PACKAGE_VERSION) \
- --console=plain
+ --console=plain \
+ --stacktrace
Review Comment:
```suggestion
--console=plain
```
##########
build/docker/README.md:
##########
@@ -180,7 +180,7 @@ Last updated: October 1, 2017
| erlang | OTP-18 | OTP-23 | |
| go | 1.15.10 | 1.16.2 | |
| haxe | 3.2.1 | 3.4.4 | THRIFT-4352: avoid 3.4.2 |
-| java | 1.8.0\_191 | 11.0.3 | |
+| java | 1.8.0\_191 | 17 | |
Review Comment:
this column is for xenial but we have deprecated it recently...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist merged pull request #2686: THRIFT-5644: java to use jdk 17 and upgrade minimal language level to 11
Posted by GitBox <gi...@apache.org>.
Jimexist merged PR #2686:
URL: https://github.com/apache/thrift/pull/2686
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jimexist commented on pull request #2686: THRIFT-5644: java to use jdk 17 and upgrade minimal language level to 11
Posted by GitBox <gi...@apache.org>.
Jimexist commented on PR #2686:
URL: https://github.com/apache/thrift/pull/2686#issuecomment-1271339060
because we want to push for raising minimal Java version to 11 we'd need to base on #2690
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org