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

Some edits for Tutorial: Shared State #768

Merged
merged 4 commits into from
Jul 10, 2024

Conversation

matildasmeds
Copy link
Contributor

  • Moved Tasks, threads and contention to the end:
    • This is deeper and more detailed than the previous sections
    • Also, the part on restructuring the code should be higher up
  • Added more warnings on Send Guards.
  • Linking to Alice Ryhl's post twice, both segments seem to benefit from this link.

This PR has some "quick fixes", that I believe would be helpful for tokio users at large.

The best thing we could do I believe, however, would be to sanitize the examples, so that all the example code would have wrapped mutexes only, making it much less likely for tokio users to end up with problematic code.

- Moved Tasks, threads and contention to the end:
  - This is deeper and more detailed than the previous sections
  - Also, the part on restructuring the code should be higher up
- Added more warnings on Send Guards.
- Linking to Alice Ryhl's post twice, both segments seem to benefit
      from this link.

This PR has some "quick fixes", that I believe would be helpful for
tokio users at large.

The best thing we could do I believe, however, would be to sanitize
the examples, so that all the example code would have wrapped mutexes
only, making it much less likely for tokio users to end up with
problematic code.
Copy link
Contributor

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small comments. Mostly I think that the heading "Most of the tiem, don't use std::sync::Mutex" is the best choice.

content/tokio/tutorial/shared-state.md Outdated Show resolved Hide resolved
content/tokio/tutorial/shared-state.md Outdated Show resolved Hide resolved
content/tokio/tutorial/shared-state.md Outdated Show resolved Hide resolved
matildasmeds and others added 2 commits July 2, 2024 18:03
Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>
Co-authored-by: Hayden Stainsby <hds@caffeineconcepts.com>
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks reasonable. I agree with @hds on the title at the top of the diff.

content/tokio/tutorial/shared-state.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great now, thanks for your changes!

@matildasmeds matildasmeds requested a review from Darksonn July 5, 2024 07:51
@hds hds merged commit 1206468 into tokio-rs:master Jul 10, 2024
7 checks passed
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.

3 participants