search

apache / iceberg-rust

Apache Iceberg
https://rust.iceberg.apache.org/
Apache License 2.0
673 stars 159 forks source link

feat: expose arrow type <-> iceberg type #637

Closed xxchan closed 2 months ago

xxchan commented 2 months ago

Previously we only exposed the schema conversion.

xxchan commented 2 months ago

I think the tests are already covered by schema conversion tests, as type conversion is just a sub-feature of schema conversion.

Xuanwo commented 2 months ago

I think the tests are already covered by schema conversion tests, as type conversion is just a sub-feature of schema conversion.

Hi, I understand that the logic might be covered by other parts, but we are adding new public APIs. How about adding unit tests directly for these new APIs?

xxchan commented 2 months ago

Ok. Do you think it would be enough to add one or two simple api call examples, or it should cover all cases like schema test?

On Fri, 20 Sep 2024 at 11:43, Xuanwo @.***> wrote:

I think the tests are already covered by schema conversion tests, as type conversion is just a sub-feature of schema conversion.

Hi, I understand that the logic might be covered by other parts, but we are adding new public APIs. How about adding unit tests directly for these new APIs?

— Reply to this email directly, view it on GitHub https://github.com/apache/iceberg-rust/pull/637#issuecomment-2362697248, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJBQZNOZSERQDYB2HSRQEI3ZXOKU3AVCNFSM6AAAAABOQEROPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRSGY4TOMRUHA . You are receiving this because you authored the thread.Message ID: @.***>

Xuanwo commented 2 months ago

Do you think it would be enough to add one or two simple api call examples

I believe that's enough.