kevinjqliu commented on code in PR #16258:
URL: https://github.com/apache/iceberg/pull/16258#discussion_r3212118936


##########
gcp/src/test/java/org/apache/iceberg/gcp/gcs/TestGcsInputStreamWrapper.java:
##########


Review Comment:
   some of these tests might be useful to keep, maybe we can rename it to 
TestAnalyticsCoreUtil?



##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,16 @@
 
 abstract class BaseGCSFile {
   private final Storage storage;
-  private final GcsFileSystem gcsFileSystem;
+  // Using Object avoid a runtime dependency on gcs-analytics-core. Cast via 
AnalyticsCoreUtil.
+  private final Object gcsFileSystem;

Review Comment:
   maybe AutoCloseable is better than Object? 



##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/BaseGCSFile.java:
##########
@@ -28,15 +27,16 @@
 
 abstract class BaseGCSFile {
   private final Storage storage;
-  private final GcsFileSystem gcsFileSystem;
+  // Using Object avoid a runtime dependency on gcs-analytics-core. Cast via 
AnalyticsCoreUtil.

Review Comment:
   ```suggestion
     // Using Object to avoid a runtime dependency on gcs-analytics-core. Cast 
via AnalyticsCoreUtil.
   ```
   nit, this and all other places too



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to