viirya commented on PR #2765: URL: https://github.com/apache/iceberg-rust/pull/2765#issuecomment-4963387565
Agreed on the direction, @mbutrovich — converging on a transform-carrying `Term` is the better end state. I checked #2665 and it is indeed turning `Term` (today just `pub type Term = Reference`) into an enum, so once that lands, a single `asc(term, null_order)` mirrors Java's actual type rather than its overload, and the `_with_transform` pair becomes redundant. Done in afe2c270: - Added the convergence note to both `asc_with_transform`/`desc_with_transform` doc comments — they're documented as the minimal non-breaking step today, expected to be deprecated in favor of Term-based `asc`/`desc` after #2665. - Hoisted the scattered in-test `use` statements to the `mod tests` top (nit 1). On nit 2 — agreed there's overlap; keeping both since the unit test pinpoints the `ActionCommit` output in isolation while the round-trip covers the serde path, and the unit test is where a failure would localize faster. Cheap to keep. -- 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]
