[GitHub] [iceberg] aokolnychyi commented on a change in pull request #3640: Core: Support case sensitivity in MergingSnapshotProducer

[GitBox <gi...@apache.org>]

aokolnychyi commented on a change in pull request #3640:
URL: https://github.com/apache/iceberg/pull/3640#discussion_r760415519



##########
File path: core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java
##########
@@ -125,6 +127,16 @@ public ThisT set(String property, String value) {
     return self();
   }
 
+  protected void caseSensitiveBinding(boolean newCaseSensitive) {

Review comment:
       It is a little bit inconsistent in the code. Some places call it `newCaseSensitive` while others `isCaseSensistive`. I went with the current naming to follow the `newXXX` pattern we use for non-boolean vars. That being said, I don't have a preference at all. Either option works for me.
   
   Yeah, the method name is to avoid conflicts. I considered calling it `caseSensitiveInternal` but not sure it is any better.




-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org