Move non-conn code out of conn.rs#2775
Merged
seanmonstar merged 3 commits intohyperium:masterfrom Apr 9, 2022
Merged
Conversation
The actual code for `Server` was previously organized very confusingly: it was thrice layered with `SpawnAll` and `Serve` which both appeared in conn.rs despite not having anything to do with the lower-level conn API. This commit changes that, removing all layering and having the code for the higher-level `Server` appear inside `server.rs` only.
`server.rs` is currently littered with `cfg`s for `http1` or `http2`, since the majority of server behaviour is only available with either one of those feature flags active. This is hard to maintain and confusing to read, so this commit extracts the two implementations into their own files, since there is very little benefit in sharing code between the two.
Member
|
Oh thanks for doing this! Yea, the server module didn't used to have separate files, and so some parts didn't get moved cleanly. I like this! |
seanmonstar
approved these changes
Apr 9, 2022
Member
seanmonstar
left a comment
There was a problem hiding this comment.
Sorry it took so long to get back to review. Wow, this is quality stuff! And I appreciate that it likely was rather tedious, thanks for cleaning this up!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The actual code for
Serverwas previously organized very confusingly: it was thrice layered withSpawnAllandServewhich both appeared in conn.rs despite not having anything to do with the lower-level conn API. This PR changes that, removing all layering and having the code for the higher-levelServerappear insideserver.rsonly.