nastra commented on PR #7992: URL: https://github.com/apache/iceberg/pull/7992#issuecomment-1642304557
> In particular, it's really unfortunate that I didn't notice that Immutables was generating the implementations in the api module until now. That means far more was exposed and will break with these changes -- otherwise we could have simply made the removed methods return null or update them to return values from the ViewVersion and kept the getters. But the implementations are changing and had public builders as well. It's a mess up on my part and I should have noticed this when `@Value.Immutable` usage was added to `iceberg-api`. I was the main driving factor to get `@Value.Immutable` into Iceberg and it should have been my responsibility to make sure we're not exposing more than we should, especially in the `iceberg-api` module. I have moved the View-specific Immutable classes as part of this PR to `iceberg-core` and I also opened https://github.com/apache/iceberg/pull/8099 to do the same for the other places in `iceberg-api`. Once both PRs are merged, I'll add a follow-up and mention in the docs that `@Value.Immutable` usage is discouraged on `iceberg-api` (with a checkstyle rule that enforces it) and will also remove the Immutable dependency -- 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]
