Skip to content

Fix typos #587

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 9, 2020
Merged

Fix typos #587

merged 3 commits into from
Dec 9, 2020

Conversation

nazar-pc
Copy link
Contributor

No description provided.

dvdplm
dvdplm reviewed Sep 21, 2020
View reviewed changes
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution.

AcqRel is a valid Ordering, can you elaborate on why SeqCst is better here?

@nazar-pc
Copy link
Contributor Author

It is not better, AcqRel is not valid for both for load and store operations. You will get panic "there is no such thing as an acquire/release load" in runtime.
It can be used for things like swap or compare_and_swap where you combine load and store in one call.
SeqCst is just a safe default, especially for examples.

niklasad1
niklasad1 approved these changes Sep 24, 2020
View reviewed changes
Copy link
Contributor

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@dvdplm dvdplm merged commit 40eec20 into paritytech:master Dec 9, 2020
@nazar-pc nazar-pc deleted the patch-1 branch December 9, 2020 11:13
This was referenced Mar 12, 2021
Merged
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@dvdplm dvdplm dvdplm approved these changes

@niklasad1 niklasad1 niklasad1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nazar-pc @dvdplm @niklasad1 @tomusdrw