feat(build): Decouple codgen from prost
#170
Conversation
tonic-build/src/client.rs
Outdated
|
|
||
| quote! { | ||
| pub async fn #ident( | ||
| &mut self, | ||
| request: impl tonic::IntoRequest<#request>, | ||
| ) -> Result<tonic::Response<#response>, tonic::Status> { | ||
| self.ready().await?; | ||
| let codec = tonic::codec::ProstCodec::default(); | ||
| let codec = tonic::codec::#codec_name::default(); |
There was a problem hiding this comment.
I was thinking we should actually make this
let codec = #codec_path::default();so that you can use any codec from any path include crates that are not tonic.
tonic-build/src/lib.rs
Outdated
| let mut stream = TokenStream::new(); | ||
|
|
||
| for comment in comments { | ||
| stream.extend(generate_doc_comment(comment.as_ref())); | ||
| for ref comment in comments { |
There was a problem hiding this comment.
| for ref comment in comments { | |
| for comment in &comments { |
There was a problem hiding this comment.
Actually comments is an iterator. So, your suggestion is not appliable straightforward. I changed the type of comments to IntoIterator.
tonic-build/src/prost.rs
Outdated
| } | ||
|
|
||
| impl schema::Context for ProstContext { | ||
| const CODEC_NAME: &'static str = "ProstCodec"; |
There was a problem hiding this comment.
so we probably want to make this non-const so that users can set it via a builder fn possibly. This path to this codec may not always be static.
tonic-build/src/schema.rs
Outdated
| } | ||
|
|
||
| pub(crate) trait Method<'a>: Commentable<'a> { | ||
| type Context: Context + Sized + 'a; |
There was a problem hiding this comment.
You don't need to add sized here iirc since its always included in a trait bound.
There was a problem hiding this comment.
I didn't specify Sized constraint. But, it is useless, unused constraint. So I removed it.
|
@LucioFranco Can you check some added commits? Which way is preferred here to fix this check failure? Adding _ prefix or disabling the function by feature gate? |
|
@Perlmint looks like there are a couple conflicts, I know some of the build code change, would you be able to update this? Then I can take another look. Thanks! |
Perlmint
force-pushed
the
feature/decouple_prost
branch
from
1ed85ab to
9881a64
Compare
|
I rebased my commits. But, It looks like I should add more commits - make types that needed to describe schema public. After pushing the commits, I'll notify. |
|
@LucioFranco Fix all checks and make possible implementing the service generator out of tonic-build. |
|
Sorry for the delay on this, I am gonna take a longer look at this once |
|
I got it. When you have time to review this, please notify me. I'll update soon :) |
|
@Perlmint Any plan to rebase this on master? :) If not I can give it a shot, but I won't be able to add commits to this MR |
|
@magnet Oh, I'm just waiting for that @LucioFranco is ready to review this. If you need, I'll do it soon. :) |
|
@Perlmint yes, sorry for the delay! We should get the ball rolling on this, looks like fbs support is almost ready :) Thanks for the patience, if we can rebase this against master then I think we can move forward. |
Perlmint
force-pushed
the
feature/decouple_prost
branch
3 times, most recently
from
c12cc97 to
1fee30e
Compare
|
@magnet rebased! |
|
Great thank you! @magnet let me know if you have any issues, I plan on taking a longer look at all of this soon. |
Prepare using tonic with other protocol buffer implement or flatbuffers. Add trait to provide needed info to the code generator. And, introduce new features to make using prost optional. Fixes: hyperium#98
Perlmint
force-pushed
the
feature/decouple_prost
branch
from
1fee30e to
f16badb
Compare
LucioFranco
changed the title
prost
Motivation
Prepare using tonic with other protocol buffer implement or flatbuffers.
Solution
Add trait to provide needed info to the code generator.
And, introduce new features to make using prost optional.