Textual corrections #25221
Textual corrections #25221
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
|
Most of this is good, but I think some was already fixed in accepted PRs, so I wonder if it will conflict. Mobile won't let me comment inline, I have one or two nits, and will try to get to a computer soon :) |
|
☔ The latest upstream changes (presumably #25218) made this pull request unmergeable. Please resolve the merge conflicts. |
| for `v2`. Which would mean two pointers to the contents of the vector on the | ||
| vector’s data, however, is stored on the [heap][sh], and so the vector contains a | ||
| pointer to that data. When we move `v` to `v2`, it creates a copy of that pointer, | ||
| for `v2`. This would mean two pointers to the content of the vector on the |
There was a problem hiding this comment.
This would mean there would be two pointers to the content of the vector on the
There was a problem hiding this comment.
I suggest "Which means that there would be two pointers to". "This" conflicts a bit with very similar phrase in the next sentence. The form of "means" refers to another Present Simple in the previous sentence. I will review the entire paragraph soon.
|
@michal-czardybon would you mind rebasing and squashing this please? we don't generally want 'merge remote-tracking branch' commits in here. now to comment inline... |
| @@ -106,8 +106,8 @@ take(v); | |||
| println!("v[0] is: {}", v[0]); | |||
| ``` | |||
|
|
|||
| Same error: “use of moved value.” When we transfer ownership to something else, | |||
| we say that we’ve ‘moved’ the thing we refer to. You don’t need some sort of | |||
| Same error: “use of moved value”. When we transfer ownership to something else, | |||
There was a problem hiding this comment.
This should actually be single curlies rather than double, would you mind fixing that?
There was a problem hiding this comment.
I am changing this. Btw. I must copy-and-paste the quote characters. Is there any way to type them directly?
There was a problem hiding this comment.
My editor is set up to insert them automatically, I'm not sure with your setup.
|
Some small nits, thank you again for this patch :) |
I will need some time to understand this. I am new to Git and was following GitHub's instructions on "push requests" without much understanding. I will read more. |
|
@michal-czardybon it's all good! I wrote a blog post about it: http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request |
|
@michal-czardybon oh, and check out the AUTHORs PR and make sure your name appears there, please |
|
(it won't yet since this wasn't merged, but it will be, so you should be on it) |
This is yet another thing I do not understand. Sorry for beginner's question by I cannot google what is "AUTHORs PR". |
|
Sorry, I'm full of shortcuts today :( Because 1.0 is happening this week, some commits are getting backported to the release. yours is one of them. I'll handle all those details, but the AUTHORS.txt file doesn't include your name, since we ran the 'get everyone's names' script before this PR was merged. So, head over to #25196 and mention that you should be in there, because of this PR. |
|
I tried to follow the instructions about "rebasing". I faced some problems, but I think I did it. Did I? |
Corrected "Ownership": - [`Variable bindings`] link was not processed properly. - Changed the paragraph about move semantics with two vectors, because it was confusing. - Removed "So it may not be as inefficient as it initially seems", because there is nothing that seems inefficient in copying pointers only. - Other text corrections. Fixed copied-and-pasted text mistakes. Revised the paragraph about moving a vector (taking into account suggestions by echochamber). Fixed markdown. Fixes requested by steveklabnik. Brought back a sentence about supposed inefficiency.
|
yes! Thanks so much |
|
@bors: r+ rollup |
|
📌 Commit bff1707 has been approved by |
…bnik I corrected some pretty obvious textual mistakes. One thing requires more attention - the paragraph at line 133 in Ownership. It was confusing, so I changed it, but I am no sure if this is what the author had in mind.
…bnik I corrected some pretty obvious textual mistakes. One thing requires more attention - the paragraph at line 133 in Ownership. It was confusing, so I changed it, but I am no sure if this is what the author had in mind.
I corrected some pretty obvious textual mistakes. One thing requires more attention - the paragraph at line 133 in Ownership. It was confusing, so I changed it, but I am no sure if this is what the author had in mind.