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

Clarifications and consistent use of quotation marks #1992

Merged
merged 6 commits into from
Oct 15, 2019

Conversation

sebras
Copy link
Contributor

@sebras sebras commented Jun 17, 2019

While translating the book to Swedish I stumbled upon a few places where it was less straightforward to translate. I looked at the original English text and felt that perhaps the original can be improved a bit. These are my suggestions.

I found that sometimes quotation marks were used around “Hello, world!”-program, but sometimes they were missing, like so: Hello, world! program. I think it may be easier to read when quotation marks are used consistently. Since I'm not translating any of the program code or strings it prints, it also make more sense this way in Swedish.

This is my first real contribution back to rust-lang/book, so if I some how prepared these commits incorrectly, let me know and I'll try to address this.

@steveklabnik
Copy link
Member

Hey @sebras !

Thanks for this PR, and sorry it took me so long to get to it.

The golden rule with PRs is basically to keep things to one logical change per PR. So while doing this isn't bad per-se, it makes it harder to merge. If you had one PR with the quotes, and then either one PR with all of the other changes, or even one PR per change, that'd be easier to get through, because you can tackle the work in bits.

I think using the "s makes sense for "hello world" here, but I'd like @carols10cents 's input.

For the other changes, some of them I think are good, some of them I'm not sure on. Carol, could you maybe review this and then I can accept or reject whatever we agree upon?

@carols10cents
Copy link
Member

Thanks! I love making the quotation marks more consistent. I also like the commit that makes our use of the word "element" clearer; I don't know what we were thinking there 😅

I do think there are two further improvements to be made here though:

The installation of Rust should also includes a copy of the documentation
locally, so you can read it offline.

I would prefer to remove "should" here, it sounds too much like "maybe it won't, sometimes the installer is flaky" which isn't what we want to convey. So I think this should read:

The installation of Rust also includes a copy of the documentation locally, so you can read it offline.

In this part:

This program creates a tuple, x, and then makes new variables for each
element by using their respective index.

I'm now thinking it should be "indices"? So:

This program creates a tuple, x, and then makes new variables for each
element by using their respective indices.

Or else singular "its" instead of "their":

This program creates a tuple, x, and then makes new variables for each
element by using its respective indices.

I'm not sure why the addition of "respective" is making my brain have a problem with the singular/plural parts. wdyt @steveklabnik ?

All the other changes are fine with me. I agree with @steveklabnik that these would have been better as multiple PRs. No problem for this time though :)

@steveklabnik
Copy link
Member

Nope, I agree 100%: I'd say it should be "indicies", though it is plural, so "their".

@sebras mind making these changes? Thanks!

@sebras
Copy link
Contributor Author

sebras commented Oct 15, 2019

The golden rule with PRs is basically to keep things to one logical change per PR.
[...]
I agree with @steveklabnik that these would have been better as multiple PRs.

I know that having logical changes split up into separate commits eases reviews.
I wasn't aware that you also prefer commits to be sent in separate PRs
(in particular for commits that don't really add any new material).

I'll update the PR shortly.

@steveklabnik
Copy link
Member

I wasn't aware that you also prefer commits to be sent in separate PRs (in particular for commits that don't really add any new material).

It's all good! In something like a book, wording change are "logical" changes, which is a bit strange. As we said, it's no big deal though :) We have had some huge PRs with tons of small changes where we wanted some, but not all, and that was tricky.

@sebras
Copy link
Contributor Author

sebras commented Oct 15, 2019

As we said, it's no big deal though :)

No worries at all. I'll try to remember next time I contribute. :)

I'd say it should be "indicies"

You tricked me a bit here! :) Supposedly the word is "indices" without the last "i". Ispell detected the new typo in Travis CI, I fix...

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Whoops! Nice catch, heh. Thanks again!

@steveklabnik steveklabnik merged commit 22397a0 into rust-lang:master Oct 15, 2019
bors added a commit to rust-lang/rust that referenced this pull request Oct 30, 2019
Update cargo, books.

## cargo

8 commits in 3ba5f27170db10af7a92f2b682e049397197b8fa..5da4b4d47963868d9878480197581ccbbdaece74
2019-10-22 15:05:18 +0000 to 2019-10-28 21:53:41 +0000
- Add --filter-platform to `cargo metadata`. (rust-lang/cargo#7376)
- Fix `cargo fix` not showing colors. (rust-lang/cargo#7550)
- Rephrase --manifest-path section (rust-lang/cargo#7409)
- Add a note to discourage the use of -Zminimal-versions. (rust-lang/cargo#7549)
- Fix profile override warning in a workspace. (rust-lang/cargo#7536)
- Fix some tests failing on Windows nightly. (rust-lang/cargo#7534)
- Show better error message for Windows abnormal termination. (rust-lang/cargo#7535)
- Run `apt update` before `apt install` (rust-lang/cargo#7541)

## reference

8 commits in 5b9d2fc..4b21b64
2019-10-03 22:39:10 +0200 to 2019-10-27 22:33:11 +0100
- Document `const_constructor` feature (rust-lang/reference#677)
- Add `non_exhaustive` to reference. (rust-lang/reference#609)
- Re-add rust-docs component for lintcheck (rust-lang/reference#702)
- group signed and unsigned integers in layout table (rust-lang/reference#700)
- Fix layout table rendering (rust-lang/reference#699)
- Add reference for attributes in function parameters (rust-lang/reference#657)
- Update now that proc macros can expand to macro_rules. (rust-lang/reference#694)
- Fix match in union example. (rust-lang/reference#684)

## book

8 commits in 9bb8b161963fcebc9d9ccd732ba26f42108016d5..28fa3d15b0bc67ea5e79eeff2198e4277fc61baf
2019-10-14 18:42:55 -0500 to 2019-10-29 07:16:09 -0500
- Update Ch19.1 on slice splitting (rust-lang/book#1999)
- fixed inconsistent terminology regarding enums (rust-lang/book#2022)
- Update ch15-03 code to match output. (rust-lang/book#2020)
- Fixes rust-lang/book#2039 (rust-lang/book#2040)
- Update ch15-03-drop.md (rust-lang/book#2049)
- unit type value is also a value (rust-lang/book#2061)
- Minor: remove an extraneous `.` (rust-lang/book#2059)
- Clarifications and consistent use of quotation marks (rust-lang/book#1992)

## rust-by-example

4 commits in 0b111eaae36cc4b4997684be853882a59e2c7ca7..f3197ddf2abab9abdbc029def8164f4a748b0d91
2019-10-14 18:34:25 -0300 to 2019-10-29 10:17:40 -0300
- Fix typos (rust-lang/rust-by-example#1285)
- Improve Cargo / Dependencies section (rust-lang/rust-by-example#1287)
- Improve Cargo / Build Scripts section (rust-lang/rust-by-example#1288)
- Make if_let exercise runnable (rust-lang/rust-by-example#1289)
@sebras sebras deleted the suggestions branch December 16, 2019 04:41
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