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

Book: Small grammatical and stylistic edits to book #34532

Merged
merged 8 commits into from Jul 3, 2016

Conversation

jonmarkprice
Copy link
Contributor

I've been reading the book and noticed a few small grammatical and stylistic issues which I've rolled into this pull request.

I'm not sure if I should do so many small, unrelated edits in a single pull request but it seems like a lot of overhead for each small edit. Maybe one commit per edit but one pull request per file/section? Feedback is very much appreciated as this is my first pull request ever!

r? @steveklabnik rollup

jonmarkprice and others added 7 commits June 27, 2016 16:34
Also "to access" is cleaner than "for accessing"
grammatical: "Here's" should be "Here are", "rules" is plural.
stylistic: "rules for" is more idiomatic than "rules about".
grammatical: No verb in "One or the other"; changed to "It's one or the other".
code: added implied `fn main() { ... }` because it is referenced in "note: previous borrow ends here"
semantic: "But" seems like the wrong word here, there is now, contrast, only further explanation. "so", "thus" or "therefor" is clearer.
grammatical: Another misuse of "Here's", should be "Here are" (or possibly "Here're").
grammatical: "use" should be capitalized. All other subheadings capitalize the first word.
two terms (input lifetime and output lifetime) so "term" needs to be plural.
no reason for a long comment in a code block when we could take it out, especially since it looks like it's using markdown (`struct`, `&` and `lvl`).
@rust-highfive
Copy link
Collaborator

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. Due to 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 the contribution instructions for more information.

@steveklabnik
Copy link
Member

Feedback is very much appreciated as this is my first pull request ever!

Thank you! It's not super important; one commit per section or one commit for the whole thing is fine.

@@ -212,7 +212,7 @@ But, unlike a move, we can still use `v` afterward. This is because an `i32`
has no pointers to data somewhere else, copying it is a full copy.

All primitive types implement the `Copy` trait and their ownership is
therefore not moved like one would assume, following the ´ownership rules´.
therefore not moved like one would assume, following the 'ownership rules'.
Copy link
Member

Choose a reason for hiding this comment

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

we use curly quotes throughout the book

@steveklabnik
Copy link
Member

Thanks so much for this! A few nits and questions, but great overall 👍

@jonmarkprice
Copy link
Contributor Author

Thanks, my bad on the curly quotes. I'll fix that.

The reason I added the main was because the book shows an error message which refers to it:

note: previous borrow ends here
fn main() {

}
^

but the code fragment in the book leaves it out:

let mut x = 5;
let y = &mut x;

*y += 1;

println!("{}", x);Run

I thought it would be clearer if the code fragment had the main function in this case because it is referenced. But obviously this is subjective.

@steveklabnik
Copy link
Member

The reason I added the main was because the book shows an error message which refers to it:

Ah, I like it! Let's keep that change then 👍

@jonmarkprice
Copy link
Contributor Author

jonmarkprice commented Jun 28, 2016

Ok I think I fixed the quotes. This is how it currently is in the book

... the ´ownership rules´. To ...

compared to different quote:

... they ‘have ownership’ of ...

So I copied that character from the second quote and put it in the markdown, it looks weird in the Github editor though...

Update: Looks like the first quotation is using &acute (U+00B4), while the second is using the standard ‘ and rsquo; (U+2018 and U+2019).

@steveklabnik
Copy link
Member

@bors: r+ rollup

looks good to me!

@bors
Copy link
Contributor

bors commented Jun 28, 2016

📌 Commit ec66b5a has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Jul 1, 2016

⌛ Testing commit ec66b5a with merge 4177cce...

@bors
Copy link
Contributor

bors commented Jul 1, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@jonmarkprice
Copy link
Contributor Author

Synchronizing submodule url for 'src/rust-installer'
fatal: Needed a single revision

command did not execute successfully: "git" "submodule" "update"
expected success, got: exit code: 1

Makefile:23: recipe for target 'all' failed
rustjob: found 0 remaining processes
Unable to find current revision in submodule path 'src/llvm'
make: *** [all] Error 1

@steveklabnik any idea why the build would fail?

@alexcrichton
Copy link
Member

@steveklabnik
Copy link
Member

@jonmarkprice if @alexcrichton 's comment wasn't clear, this was a failure that was 100% not your fault; a bug in our automation.

@bors
Copy link
Contributor

bors commented Jul 3, 2016

⌛ Testing commit ec66b5a with merge 5e858f3...

bors added a commit that referenced this pull request Jul 3, 2016
Book: Small grammatical and stylistic edits to book

I've been reading [the book](https://doc.rust-lang.org/book/) and noticed a few small grammatical and stylistic issues which I've rolled into this pull request.

I'm not sure if I should do so many small, unrelated edits in a single pull request but it seems like a lot of overhead for each small edit. Maybe one commit per edit but one pull request per file/section? Feedback is very much appreciated as this is my first pull request ever!

r? @steveklabnik rollup
@bors bors merged commit ec66b5a into rust-lang:master Jul 3, 2016
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

5 participants