Skip to content
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

chore(examples): Call Routes::prepare #1857

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

tottoto
Copy link
Collaborator

@tottoto tottoto commented Aug 6, 2024

Calls Routes::prepare in examples.

Verified

This commit was signed with the committer’s verified signature.
alexanderniebuhr Alexander Niebuhr
@djc
Copy link
Contributor

djc commented Aug 6, 2024

Why? What does it do/why is it valuable? If it works without it, it looks like pure noise.

@tottoto
Copy link
Collaborator Author

tottoto commented Aug 6, 2024

No, it is not noise. This is recommended because it uses axum's router internally. Examples should show how it should be used.

@djc
Copy link
Contributor

djc commented Aug 7, 2024

You haven't answered my question. I mean "noise" not in a disparaging but in a technical sense, as in, code that is unnecessary (since it apparently compiles just fine without it). So why is it better to call prepare() on these?

@tottoto
Copy link
Collaborator Author

tottoto commented Aug 8, 2024

I think I have already answered all of your question. According to your comment it appears that you believe that examples should be judged solely on whether they compile, but I do not agree. Even if the code can be compiled without a certain step being done, when that step is recommended it should be showed in the example. My opinion is that one of the role of examples is to show recommended usage of the APIs, not just to make them compile.

So why is it better to call prepare() on these?

As I have already answered, this is because tonic uses axum internally as library and this processing is recommended by axum in the use case of tonic doing. Therefore, tonic's router inherits axum's recommendation and also recommends users to run this process. It is not necessary but recommended.

@djc
Copy link
Contributor

djc commented Aug 8, 2024

Okay, can you link that recommendation here? Essentially I'm asking why Axum recommends this.

@tottoto
Copy link
Collaborator Author

tottoto commented Aug 8, 2024

@djc djc added this pull request to the merge queue Aug 23, 2024
Merged via the queue into hyperium:master with commit 4c9356c Aug 23, 2024
16 checks passed
@tottoto tottoto deleted the call-routes-prepare-in-example branch August 23, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants