ashvina commented on code in PR #653:
URL: https://github.com/apache/incubator-xtable/pull/653#discussion_r1960954090
##########
xtable-api/src/main/java/org/apache/xtable/model/storage/FilesDiff.java:
##########
@@ -92,12 +92,12 @@ public static <L, P> FilesDiff<L, P> findNewAndRemovedFiles(
* @param <P> the type of the previous files
* @return the set of files that are added
*/
- public static <P> FilesDiff<InternalDataFile, P> findNewAndRemovedFiles(
+ public static <P> FilesDiff<? extends InternalBaseFile, P>
findNewAndRemovedFiles(
Review Comment:
> Why wouldn't it be a good use of an interface?
`InternalFile` is a data class. If the data class is simply a container for
data and doesn't require any specific behavior or implementation, then using an
interface is uncommon
> Currently we are always casting to a specific instance
I wouldn't use call it always, but anyway.
Well currently none of the sources and target are handling any other kind of
file type. There are pending PRs for that and new ones will be needed in the
future.
> instead of leveraging the shared implementation of the base class.
That will not happen even with interface. Each target and source will handle
each type of file differently.
For now, I’m going to abandon this PR as I don’t see a clear path to
resolving this issue. If anyone has suggestions or ideas on how to tackle this,
I’m all ears!
--
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]
