the-onewho-knocks commented on code in PR #1321:
URL: https://github.com/apache/iceberg-go/pull/1321#discussion_r3491517768
##########
manifest.go:
##########
@@ -1274,6 +1274,16 @@ func WithManifestFileContent(content ManifestContent)
ManifestFileOption {
}
}
Review Comment:
agreed I'll go with WriteManifestV3(...) (ManifestFile, int64, error). A
dedicated v3 entrypoint makes the reconstruction only use case explicit at the
call site, rather than relying on a doc contract for a general option that
looks identical to WithManifestFileContent but carries this footgun. Neither
shape changes where the actual safety boundary lives that's still
AddManifests's nil-check but this makes accidental misuse much less likely.
Will drop WithManifestFileFirstRowID and rework as described below
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
