the-other-tim-brown commented on code in PR #569: URL: https://github.com/apache/incubator-xtable/pull/569#discussion_r1865045181
########## xtable-core/src/main/java/org/apache/xtable/delta/DeltaConversionTarget.java: ########## @@ -215,13 +224,64 @@ public String getTableFormat() { return TableFormat.DELTA; } + @Override + public Optional<String> getTargetCommitIdentifier(String sourceIdentifier) { Review Comment: If there is no commit Identifier in the latest commit, should this just return an empty Optional? ########## xtable-api/src/main/java/org/apache/xtable/spi/sync/ConversionTarget.java: ########## @@ -76,8 +76,9 @@ public interface ConversionTarget { * Starts the sync and performs any initialization required * * @param table the table that will be synced + * @param sourceIdentifier */ - void beginSync(InternalTable table); + void beginSync(InternalTable table, String sourceIdentifier); Review Comment: I am wondering if it makes sense to attach the source format as part of the identifier. Then we can also have some metadata about what the writer format was. -- 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: commits-unsubscr...@xtable.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org