[GitHub] [iceberg] kbendick commented on a change in pull request #1530: Spark: Implement equals and hashCode in SparkTable

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

kbendick commented on a change in pull request #1530:
URL: https://github.com/apache/iceberg/pull/1530#discussion_r498005000



##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
##########
@@ -525,6 +525,34 @@ public void testVersionHintFileMissingMetadata() throws Exception {
         () -> TABLES.load(tableLocation));
   }
 
+  @Test
+  public void testTableEquality() throws Exception {

Review comment:
       Throwing in my +1 for adding `name` to the API. But I'd hope that we can still leave a meaningful enough `toString` method in place.
   
   Without it, we just wind up with your typical memory location etc, specifically when / if `Table` is used in a parameterized test (I just spent half my evening adding parameterized names to all of the tests that didn't have them).
   
   What is everybody's opinion on implementing `toString` when it's not strictly necessary? Would you say it's a good practice, or is it possibly just extra unused code that needs to be maintained over time? I know this is potentially more of a religious question than a hard and fast rule, but I'd love to hear your input.




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

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