ctubbsii commented on code in PR #2606:
URL: https://github.com/apache/thrift/pull/2606#discussion_r871996671


##########
lib/java/src/crossTest/java/org/apache/thrift/test/TestClient.java:
##########
@@ -823,4 +808,10 @@ public static void main(String[] args) {
 
     System.exit(returnCode);
   }
+
+  private static byte[] getBytesData() {
+    ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream(256);
+    IntStream.range(-128, 128).forEach(byteArrayOutputStream::write);
+    return byteArrayOutputStream.toByteArray();
+  }

Review Comment:
   These seem unrelated, but it's a nice improvement. None of this requires 
Java 11, right?



##########
lib/java/gradle/sourceConfiguration.gradle:
##########
@@ -21,21 +21,28 @@
 // ----------------------------------------------------------------------------
 // Compiler configuration details
 
-// These two properties are still needed on JDK8, and possibly used directly by
-// plugins. However, the '--release' option added below makes these two
-// properties redundant when building with JDK9 or later.
-sourceCompatibility = '1.8'
-targetCompatibility = '1.8'
+// We are using Java 11 toolchain to compile.
+// This enables decoupling from the Java version that gradle runs, from
+// the actual JDK version for the project. For more details, see
+// https://docs.gradle.org/current/userguide/toolchains.html
+//
+// The '--release' option added below makes sure that even if we are using
+// the toolchain version > 8, the final artifact is at version 8. There is
+// also a runtime CI that's based on Java 8 to ensure that.
+java {
+    toolchain {
+        languageVersion = JavaLanguageVersion.of(11)
+    }
+}
 
-tasks.withType(JavaCompile) {
+tasks.withType(JavaCompile).configureEach {
     options.encoding = 'UTF-8'
     options.debug = true
     options.deprecation = true
+    assert JavaVersion.current().isJava11Compatible(), "we are using Java 11 
to compile, this is to ensure that"

Review Comment:
   ```suggestion
       assert JavaVersion.current().isJava11Compatible(), "JDK 11 or higher is 
required to build"
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to