kbendick edited a comment on pull request #1991: URL: https://github.com/apache/iceberg/pull/1991#issuecomment-751543148
I've added a new test to ensure doc comments are preserved in a round trip conversion. The other test that was touched, `TestMergeAppend#testManifestMergeMinCount`, needed to be updated to account for the slight increase in manifest file size due to the field doc comments that now appear one time in the header. I also updated the language surrounding the choice of the new `commit.manifest.target-size-bytes` used in the test to account for both v1/v2 sizes. I tried to make the language match the style of what was already there, but feel free to suggest something simpler. Given that the header is only once per manifest file and the default target size is 8mb, the small increase in manifest file size is well worth the benefit of keeping the field docs. I will let others weigh in on that if they are so inclined. ---------------------------------------------------------------- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
