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

More edits to the async chapter #4033

Open
wants to merge 28 commits into
base: only-new-async
Choose a base branch
from
Open

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Sep 13, 2024

Important

This PR is the home of the next set of edits targeting #3909. It is not really intended for public review, but rather as an easy place for @carols10cents and me to be able to discuss or coordinate if/as makes sense to us. Thanks!

This has nearly all the review comments from #3909 integrated. In particular, beyond clarifications and stylistic improvements based on review comments and my own detailed edit, it also includes:

  • A fairly significant restructuring of the material based on those initial rounds of feedback. Basically all of the discussion of the actual trait definitions is moved to a new penultimate section in the chapter, and the two earlier sections covering the join, race, and so on have been combined. All the original material is still there, just rearranged (and much clarified).
  • Diagrams! The opening section now has some Graphviz-generated SVGs to illustrate the difference between parallelism and concurrency.

As of the time I am actually opening this PR, the remaining tasks are:

  • Rewrite the opening of section 17.1 to use a short code scraping example I came up with, which demonstrates and motivates all the concepts and syntax much more thoroughly than the terrible example it currently opens with.
  • Add one or more diagrams to the section on pinning so the memory layout can be easier for people to grok.
  • Add a couple code samples to the pinning section to avoid the wall-of-text problem Will identified in the first round of reviews!

I expect to finish those up by midway through next week. 🎉

chriskrycho and others added 26 commits August 24, 2024 08:19
- Set their view boxes to the original height and width, so they are
  guaranteed to present correctly.
- For Figure 17-02, use the trick of adding a hidden node and hidden
  arrow to it in “Task 2” to align the two boxes.
- Create a section (which will be deleted or at least reintegrated once
  all is said and done) to hold content pulled out of other sections for
  the sake of clearer flow and understanding.
- Pull “advanced” material from 17.00, 17.01, and 17.02 into the holding
  section and start reorganizing their content to account for shifting
  around materials.
In addition to the baseline changes, skip over non-directory code where
directories are needed to deal with things like `.DS_Store` files. Also
add a bunch of context on error causes from `std::io::Error` because it
was *impossible* to figure out exactly what the source of those were.
Use immutable borrow of `TcpStream` when creating `BufReader`
Add `cargo init` usage suggestion to 1.3
Update build instructions: include mdbook plugins
The `block_on` name is what both Tokio and smol use, but it is a bit
obscure from the point of view of introducing this material. `run` says
much more clearly what it does *for the level we care about here*, I
think.
These make up *most* of the rest of the edits I caught while rereading
which are not *major structural revisions*, along with some of the bits
required for those major structural revisions.
This does *not* yet incorporate any of the relevant feedback from Carol
on this, so a couple spots are still pretty messy.
Along with the wording and phrasing-level edits, pull out a fair bit of
material for the “advanced” section at the end, specifically the details
of what `Stream` and `StreamExt` actually do.
Bonus: fix some style guide issues, too!

Co-authored-by: Carol (Nichols || Goulding) <carol.nichols@gmail.com>
Co-authored-by: James Munns <james@onevariable.com>
Co-authored-by: Tim McNamara <paperless@timmcnamara.co.nz>
Co-authored-by: James Munns <james@onevariable.com>
Co-authored-by: Tim McNamara <paperless@timmcnamara.co.nz>
Co-authored-by: Carol (Nichols || Goulding) <carol.nichols@gmail.com>
@chriskrycho chriskrycho changed the base branch from main to only-new-async September 13, 2024 22:34
I accidentally copied these in when pulling in the `trpl-note` mdbook
preprocessor many months ago, and we did not notice amidst the many
other changes in that PR!
@chriskrycho
Copy link
Contributor Author

@makarichevss thanks, but we are not looking for general public feedback at present. This chapter is still very much a work in progress.

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.

4 participants