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

Mention Option::take() as an alternative to mem::replace() #33

Merged
merged 2 commits into from
Oct 10, 2016
Merged

Mention Option::take() as an alternative to mem::replace() #33

merged 2 commits into from
Oct 10, 2016

Conversation

partim
Copy link
Contributor

@partim partim commented Oct 9, 2016

I’m not sure if this mention should go in the Motivation or Discussion sections. Since Option is being mentioned in the Motivation, however, I added it right there.

@@ -53,6 +53,11 @@ result, we get the original `name` *as an owned value*. We can wrap this in
an `Option` or another enum that we can destructure in the next step to put the
contained values into our mutably borrowed enum.

Note, however, that we you are using an `Option` and are replacing its
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: "we you" should be "when you"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Fixed and hopefully improved a little overall.

@lambda-fairy
Copy link
Collaborator

👍

@lambda-fairy lambda-fairy merged commit 9d95504 into rust-unofficial:master Oct 10, 2016
@partim partim deleted the take-for-replace branch October 10, 2016 09:00
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.

None yet

2 participants