rdblue commented on a change in pull request #3959:
URL: https://github.com/apache/iceberg/pull/3959#discussion_r805436120



##########
File path: core/src/main/java/org/apache/iceberg/MetricsConfig.java
##########
@@ -127,24 +135,35 @@ public static MetricsConfig forPositionDelete(Table 
table) {
     return new MetricsConfig(columnModes.build(), defaultMode);
   }
 
-  private static MetricsConfig from(Map<String, String> props, SortOrder 
order) {
+  /**
+   * Generate a MetricsConfig for all columns based on overrides, sortOrder, 
and defaultMode.
+   * @param props will be read for metrics overrides 
(write.metadata.metrics.column.*) and default
+   *              (write.metadata.metrics.default)
+   * @param order sort order columns, will be promoted to truncate(16)
+   * @param defaultMode default, if not set by user property
+   * @return metrics configuration
+   */
+  private static MetricsConfig from(Map<String, String> props, SortOrder 
order, String defaultMode) {

Review comment:
       Instead of renaming the variable that is used everywhere, I'd just 
rename the incoming argument. There would be fewer changes if this used 
`defaultDefaultMode` or something.




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