Samrat002 commented on PR #21666: URL: https://github.com/apache/flink/pull/21666#issuecomment-1407279532
hi @dannycranmer Thank you for reviewing the change. I am completely aligned with you and where you are coming from. I felt this change will simplify tests while working on tests for `GlueCatalog` Here are my few thoughts why i think this change will make sense in this case 1. `CatalogTest` also contains `checkEquals` methods e.g `checkEquals(CatalogColumnStatistics cs1, CatalogColumnStatistics cs2) ` which makes the structure of code not aligned properly. `checkEquals` methods are scattered over 2 different classes. 2. `CatalogTestUtil` class contains only overloaded methods of `checkEquals`. These methods can only be used with utility methods in `CatalogTest`. If you see the code wherever it is used , so if you want to reuse `checkEquals` in other place , i think you need to have reference to `CatalogTest` and call static methods from `CatalogTestUtils` 3. Contributor who added `CatalogTestUtil` marked this class to remove as todo [here](https://github.com/apache/flink/pull/21666/files#diff-8247b04d29d9f57d0445fb65ff23cc92efbb6dc1f929c8004efe167c2ea6fb21L41) (😅 ignore this one just gathering more support) -- 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]
