Various improvements #43
Closed
Conversation
This file contains 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
“not ready” doesn’t add much
This also changes to use mio::Events::iter and moves dispatch to the Inner part of the core (and changes mutability to &self).
Done by rustfmt.
In some cases rustfmt created rather strange looking code, so I’ve excluded that from the commit.
- Add links to the futures, mio and tokio-uds crates. - Add links to various structs and types mentioned. - use eprintln for error reporting in the example.
In the old version to code flow was somewhat hard to follow, and even though the new version uses more lines of code I believe it’s easier to follow.
- Fixed links usage. - Removed references to a no longer existing `Window` struct. - Made notes about using functions in context of a future.
- Since HTTP uses TCP (QUIC aside) using it as an example in an UDP protocol feels wrong. - Make the note of tampering with the underlying streams more explicit.
Adds an explanation of every public struct.
Adds an explanation of every public struct and how they work together.
Reorder the various option methods; get first then set. Note about panicing added to poll_read.
Makes it easier to follow and well as reducing the number of lines of code.
Avoid an allocation.
Start testing on OS X. Print the version of cargo and rustc at the start of the testing, useful for testing with nightly. Add cargo to the cache. Enable the verbose flag while building and testing.
|
I also made a number of notes about some possible improvement that would break the API. Should a create an issue for each of them (about 6 or so) or 1 big issue? |
|
Hi, thanks for the PR! This seems to be conflating a bunch of things into a one PR. It makes it hard to tell exactly what is going on. It would probably be easier to approach this into more granular PRs. Each PR should make a logical change. This makes it easier to tell how the various changes relate. Also, please do not run |
|
Ok will do, might take some time. |
|
I've create 5 pull requests instead. |
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.
I starting review the tokio crates and in doing make a bunch of cleanups commits. Some hightlights:
try!with?.Err(io::ErrorKind::WouldBlock.into()).