hililiwei commented on PR #4325: URL: https://github.com/apache/iceberg/pull/4325#issuecomment-1186144217
> This looks really close to me, the only issues I think are remaining are > > 1. We cannot change any deprecated apis > 2. Any public apis that are changed need to remain, additional apis can be added and the originals can be deprecated > 3. I'm not sure about where the Action Constants should go. I think the SparkActionOptions is maybe too general but is probably fine if we document it with some Javadocs. As Is the option is there but there is no way of determining which actions it applies too. Ideally I would have had Migrate + Snapshot inherit from something with the option. It seems like putting it API would probably be too much, but maybe we just add an interface that they both can inherit SparkCreateAction, which has the option as a public field? I added some docs to `SparkActionOptions`, and I originally wanted to move all the Spark action options to it (not in this PR). I found that we already have `BaseSparkAction` , maybe we can put these options in it (i'm not sure)? -- 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]
