kbendick commented on code in PR #5118:
URL: https://github.com/apache/iceberg/pull/5118#discussion_r904530692


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -133,7 +133,8 @@ public static class SnapshotLogEntry implements 
HistoryEntry {
     private final long timestampMillis;
     private final long snapshotId;
 
-    SnapshotLogEntry(long timestampMillis, long snapshotId) {
+    // VisibleForTesting
+    public SnapshotLogEntry(long timestampMillis, long snapshotId) {

Review Comment:
   This is one I could make public via reflection, as the class itself is 
`public static`.



##########
core/src/main/java/org/apache/iceberg/TableMetadataParser.java:
##########
@@ -83,30 +83,30 @@ private TableMetadataParser() {
   }
 
   // visible for testing
-  static final String FORMAT_VERSION = "format-version";
-  static final String TABLE_UUID = "table-uuid";
-  static final String LOCATION = "location";
-  static final String LAST_SEQUENCE_NUMBER = "last-sequence-number";
-  static final String LAST_UPDATED_MILLIS = "last-updated-ms";
-  static final String LAST_COLUMN_ID = "last-column-id";
-  static final String SCHEMA = "schema";
-  static final String SCHEMAS = "schemas";
-  static final String CURRENT_SCHEMA_ID = "current-schema-id";
-  static final String PARTITION_SPEC = "partition-spec";
-  static final String PARTITION_SPECS = "partition-specs";
-  static final String DEFAULT_SPEC_ID = "default-spec-id";
-  static final String LAST_PARTITION_ID = "last-partition-id";
-  static final String DEFAULT_SORT_ORDER_ID = "default-sort-order-id";
-  static final String SORT_ORDERS = "sort-orders";
-  static final String PROPERTIES = "properties";
-  static final String CURRENT_SNAPSHOT_ID = "current-snapshot-id";
-  static final String REFS = "refs";
-  static final String SNAPSHOTS = "snapshots";
-  static final String SNAPSHOT_ID = "snapshot-id";
-  static final String TIMESTAMP_MS = "timestamp-ms";
-  static final String SNAPSHOT_LOG = "snapshot-log";
-  static final String METADATA_FILE = "metadata-file";
-  static final String METADATA_LOG = "metadata-log";
+  public static final String FORMAT_VERSION = "format-version";
+  public static final String TABLE_UUID = "table-uuid";
+  public static final String LOCATION = "location";
+  public static final String LAST_SEQUENCE_NUMBER = "last-sequence-number";
+  public static final String LAST_UPDATED_MILLIS = "last-updated-ms";
+  public static final String LAST_COLUMN_ID = "last-column-id";
+  public static final String SCHEMA = "schema";
+  public static final String SCHEMAS = "schemas";
+  public static final String CURRENT_SCHEMA_ID = "current-schema-id";
+  public static final String PARTITION_SPEC = "partition-spec";
+  public static final String PARTITION_SPECS = "partition-specs";
+  public static final String DEFAULT_SPEC_ID = "default-spec-id";
+  public static final String LAST_PARTITION_ID = "last-partition-id";
+  public static final String DEFAULT_SORT_ORDER_ID = "default-sort-order-id";
+  public static final String SORT_ORDERS = "sort-orders";
+  public static final String PROPERTIES = "properties";
+  public static final String CURRENT_SNAPSHOT_ID = "current-snapshot-id";
+  public static final String REFS = "refs";
+  public static final String SNAPSHOTS = "snapshots";
+  public static final String SNAPSHOT_ID = "snapshot-id";
+  public static final String TIMESTAMP_MS = "timestamp-ms";
+  public static final String SNAPSHOT_LOG = "snapshot-log";
+  public static final String METADATA_FILE = "metadata-file";
+  public static final String METADATA_LOG = "metadata-log";

Review Comment:
   I don't have concern making these public personally as they are generally 
speaking usable by engines if needeed, but open to other opinions.



##########
core/src/main/java/org/apache/iceberg/BaseSnapshot.java:
##########
@@ -32,7 +32,8 @@
 import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 
-class BaseSnapshot implements Snapshot {
+// visible for testing
+public class BaseSnapshot implements Snapshot {

Review Comment:
   I don't love these changes, but they are in `core`.



##########
core/src/test/java/org/apache/iceberg/LocalTableOperations.java:
##########
@@ -27,13 +27,13 @@
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.junit.rules.TemporaryFolder;
 
-class LocalTableOperations implements TableOperations {
+public class LocalTableOperations implements TableOperations {

Review Comment:
   This is in test so I don't personally have concerns making it public, but 
others might feel differently.



##########
api/src/main/java/org/apache/iceberg/PartitionSpec.java:
##########
@@ -102,7 +102,8 @@ public boolean isUnpartitioned() {
     return !isPartitioned();
   }
 
-  int lastAssignedFieldId() {
+  // visible for testing
+  public int lastAssignedFieldId() {

Review Comment:
   This is in the api module, so we might choose simply to not test this 
instead (or access the data by other means).



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