tomtongue commented on code in PR #10063: URL: https://github.com/apache/iceberg/pull/10063#discussion_r1547639555
########## data/src/test/java/org/apache/iceberg/io/TestPositionDeltaWriters.java: ########## @@ -20,43 +20,42 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.util.Arrays; import java.util.List; import org.apache.iceberg.DataFile; import org.apache.iceberg.DeleteFile; import org.apache.iceberg.FileFormat; +import org.apache.iceberg.Parameter; +import org.apache.iceberg.ParameterizedTestExtension; +import org.apache.iceberg.Parameters; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.RowDelta; import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.util.StructLikeSet; import org.junit.Assert; Review Comment: @nastra Thanks for the review. I only migrate classes related to `TableTestBase` to delete the class, so I keep these classes JUnit4 because there should be a lot of changes if each class is updated to JUnit5. And, I'm thinking that after deleting `TableTestBase`, I'll migrate those classes to Junit 5. Should I update all classes in this PR to JUnit 5? or partially update? ########## data/src/test/java/org/apache/iceberg/io/TestPositionDeltaWriters.java: ########## @@ -20,43 +20,42 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.util.Arrays; import java.util.List; import org.apache.iceberg.DataFile; import org.apache.iceberg.DeleteFile; import org.apache.iceberg.FileFormat; +import org.apache.iceberg.Parameter; +import org.apache.iceberg.ParameterizedTestExtension; +import org.apache.iceberg.Parameters; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.RowDelta; import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.util.StructLikeSet; import org.junit.Assert; Review Comment: @nastra Thanks for the review. This time, I only migrate classes related to `TableTestBase` to delete the class, so I keep these classes JUnit4 because there should be a lot of changes if each class is updated to JUnit5. And, I'm thinking that after deleting `TableTestBase`, I'll migrate those classes to Junit 5. Should I update all classes in this PR to JUnit 5? or partially update? -- 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
