Use a published version of the rust-proto branch #31
Conversation
This uses the same code as before, but uses it via crates.io, rather than Git. Signed-off-by: Nick Cameron <nrc@ncameron.org>
| @@ -16,8 +16,8 @@ prost-codec = ["syn", "quote", "prost-build"] | |||
| grpcio-prost-codec = ["grpcio-compiler/prost-codec", "prost-codec"] | |||
|
|
|||
| [dependencies] | |||
| protobuf = { version = "2", optional = true } | |||
| protobuf-codegen = { version = "2", optional = true } | |||
| protobuf = { package = "nrc-protobuf", version = "2", optional = true } | |||
There was a problem hiding this comment.
I feel worried about it, especially when using protobuf-build in grpc-rs. What's the problem of keeping using a branch?
There was a problem hiding this comment.
It means we can't publish kvproto, which blocks us publishing the Rust client.
I think I must make it a feature in any case because of grpcio-compiler.
There was a problem hiding this comment.
it's a minor fork of rust-proto to use Prost naming conventions and adding support for some TiKV-specific pretty printing
There was a problem hiding this comment.
I don't feel it right to let rust client to depend on a forked version of rust-proto either.
For "TiKV-specific thing", we can leave it inside TiKV project. For naming conventions, we better discussing with rust-protobuf project and get it merged into the project instead of creating a new one.
There was a problem hiding this comment.
I don't think they will accept the naming changes - it would be a big breaking change. OTOH, not using those changes makes supporting both Prost and rust-proto very messy.
I agree it is sub-optimal to use a forked version, but we are already doing that, the only change here is to use a published version of the fork.
And to clarify, if we want to publish KvProto, then we can't depend on Git repos - it is not allowed by crates.io. Whereas depending on a fork is sub-optimal, but not impossible (and we already use a forked version of GRPC among other things, which seems worse to me).
There was a problem hiding this comment.
rust-protobuf supports customization, I don't think it has to be a breaking change.
Forked version of gRPC is not the same use case here. Almost all changes to the forked are just bugfix cherry-picks, we don't mean to introduce any custom features to the forked version. And it will not introduce a different crate or make different types.
This uses the same code as before, but uses it via crates.io, rather than Git.
PTAL @BusyJay @breeswish