RussellSpitzer commented on code in PR #12066: URL: https://github.com/apache/iceberg/pull/12066#discussion_r1930808394
########## core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java: ########## @@ -51,6 +50,8 @@ public class RewriteTablePathUtil { private static final Logger LOG = LoggerFactory.getLogger(RewriteTablePathUtil.class); + // iceberg client file separator, independent of that of the underlying file system Review Comment: I'm not sure this comment is helpful? We don't have an actual defined "Iceberg Client File Separator". This is a POSIX path separator that makes sense to use because AWS, GCP, AZURE, HDFS etc ... all use it. Comments shouldn't beg the question, they should ask and answer it if needed. In this case something like "Use the POSIX separator instead of File.separator because File.separator is dependent on the client environment and not the target filesystem. POSIX is compatible with S3, GCS, HDFS, etc ..." So we describe what we are doing and why we are doing that instead of something else. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org