-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inline Generated Thift Code Into Parquet Crate #2502
Comments
We've had this discussion before, and I support this approach (sunchao/parquet-format-rs#10). I think what could also bolster the argument for making @sunchao @tustvold I'd be in favour of deprecating and eventually archiving |
I also agree that incorporating the code generated from thrift into the parquet crate and removing the dependency on parquet-format would be a good idea because:
|
+1. Let me know if you need any help.
I'd prefer to do the deprecating & archiving later, since we have internal projects that are still using |
Automatically added labels {'api-change'} from #2626 |
Is your feature request related to a problem or challenge? Please describe what you are trying to do.
Currently parquet-format, which contains the generated code from the parquet thrift definitions, is published as a separate crate from a separate repo
There is discussion about potentially donating parquet-format and I wanted to create an issue to discuss this
Describe the solution you'd like
It's necessarily an incomplete picture, but looking at the reverse dependencies of parquet-format they all already have a dependency on
parquet
. As such I would like to propose inlining the generated code in aformat
module withinparquet
. This would avoid maintenance burden associated with an additional crateDescribe alternatives you've considered
We could not do this or we could donate the code, but still vend it as a separate crate. This would not address the maintenance burden of additional dependencies, but perhaps updates are infrequent enough that this doesn't matter
Additional context
If we are happy to proceed with this, I would be willing to help with the mechanics of moving the code across
FYI @sunchao @nevi-me
The text was updated successfully, but these errors were encountered: