lidavidm commented on code in PR #3714: URL: https://github.com/apache/arrow-adbc/pull/3714#discussion_r2591547170
########## rust/core/src/non_blocking/stream.rs: ########## Review Comment: Does arrow-rs not already have a trait for this? ########## rust/core/src/options.rs: ########## Review Comment: Maybe not necessary to update these docstrings? ########## rust/core/src/schemas.rs: ########## Review Comment: Ditto here ########## rust/core/src/executor.rs: ########## Review Comment: nit, but I think it makes more sense to keep all the sync-async bridge code in one module rather than split off a single trait into one module and mix sync and async code in sync.rs ########## rust/core/src/non_blocking/local.rs: ########## Review Comment: I think there needs to be explanation somewhere of what "local" means (traits that are not Send?) Additionally, does it make sense to have non-Send variants? It's not clear to me that that's useful (when would a connection be limited to a single thread?) -- 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]
