crepererum commented on PR #638: URL: https://github.com/apache/arrow-rs-object-store/pull/638#issuecomment-3889833106
Inspired by the proposed table, I think the options on the enum could be documented better: - examples could be included - the explanations proposed here are a bit more detailed That concerns the enum(s) alone though. I'm not entirely sure if the table is really worth it though, because: - **out-of-sync risk:** It's a manual, write-once copy of the enum ground truth. This is gonna be out-of-sync fast and keeping it in-sync is gonna be a substantial maintenance burden. This could be worked around by do some macro magic, similar to DataFusion's config system ([ref1](https://docs.rs/datafusion-common/latest/datafusion_common/macro.config_namespace.html), [ref2](https://docs.rs/datafusion-common/latest/src/datafusion_common/config.rs.html#1455-1486), [ref3](https://docs.rs/datafusion-common/latest/src/datafusion_common/config.rs.html#1489-1513)). That however would also add complexity and I don't think we have the same, nested config structure that DataFusion requires. - **readability:** I'm not sure if the table is really more readable. Looking at the enum, I also get data types and the occasional "warning" block. Due to the limited nature of markdown, this is gonna be hard to replicate in a table view. -- 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]
