-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(docs): Add routeguide tutorial #21
Conversation
Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
This is looking really good! Let me know when you'd like a review pass and if you need anything! <3 |
Thanks! I think it's ready for review up to the |
@LucioFranco This is ready for the first review round. |
@alce thank you so much for doing this. I am a bit short on time today so will try to get through this this week. Thanks <3 |
cc @bIgBV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an awesome guide! I've left some comments, but they're mostly nits.
```shell | ||
$ cargo run --bin routeguide-client | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a small line about what the user should expect to see at this step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bIgBV thank you for your comments.
I've added a small explanation of what the user should see
convenient because once we've set everything up, there is no extra step to keep the generated code | ||
and our `.proto` definitions in sync. | ||
|
||
Behind the scenes, Tonic uses [PROST!] to handle protocol buffer serialization and code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the link text supposed to be capitalized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is how they write the project's name. It does look weird though.
let (mut tx, rx) = mpsc::channel(4); | ||
let features = self.features.clone(); | ||
|
||
tokio::spawn(async move { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a great way to use the async-stream
crate right? Maybe mention it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async-stream is used on the bidirectional streaming example and we talk abut it briefly. Do you think we should use it here too or explain in a bit more detail what it does?
@alce just wanna check in, is this complete or is there more you wanted to add? |
I think this is ready for now. We'll need to tweak a couple of things for the next tonic release but it should be ready to go if you agree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alce I just want to say thank you for bearing with me through all this and thank you so much for contributing this. Docs and tutorials is one of those things that differentiate projects so I really appreciate this contribution! <3
A work in progress tutorial for the routeguide example