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