Sober7135 commented on PR #811: URL: https://github.com/apache/incubator-graphar/pull/811#issuecomment-3674139910
any updates on this PR? My upcoming Rust bindings will rely on the enums in `fwd.h`, so I’m tracking this closely. I also have a few suggestions to share: 1. Could we also specify an explicit underlying type for the other enums in `fwd.h` (e.g., `Cardinality`, `FileType`) for consistency? 2. Would you consider renaming the PR to something more specific, e.g. **`feat(c++): specify underlying types for enums in fwd.h`**? The current title feels a bit general. 3. I think we should align on a convention for underlying types (`int32_t` vs `uint32_t` vs something else). My preference is **`uint32_t`** since these enums are never negative. 4. For `AdjListType`, should we switch the underlying type to **`uint32_t`** (or `int32_t`) to match the agreed convention? I tend to use **32-bit width** for consistency. Using `uint8_t` does not seem to yield significant benefits. Thoughts? CC @yangxk1 @shivendra-dev54 -- 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]
