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

Minor doc fixes in "Crates and Modules" and "Lifetimes" chapters #32634

Merged
merged 2 commits into from
Apr 6, 2016

Conversation

varunvats
Copy link

These commits fix a couple of (minor) issues in the Crates and Modules and the Lifetimes chapters of the book.

r? @steveklabnik

@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.

`src/japanese/farewells/mod.rs`. Because these sub-modules don’t have
their own sub-modules, we’ve chosen to make them
`src/english/greetings.rs`, `src/english/farewells.rs`,
`src/japanese/greetings.rs` and `src/japanese/farewells.rs`. Whew!
Copy link
Member

Choose a reason for hiding this comment

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

why was this change made?

Copy link
Author

Choose a reason for hiding this comment

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

The english and japanese modules each contain two sub-modules - greetings and farewells. These means that the compiler must look for a total of eight different module files - four that reside directly under the english and japanese directories - src/{english,japanese}/{greetings,farewells}.rs and four (mod.rs files) that reside under the greetings and farewells directories - src/{english,japanese}/{greetings,farewells}/mod.rs. The documentation listed only four of these - src/{english,japanese}/greetings.rs and src/{english,japanese}/farewells/mod.rs, which confused me. I thought it would be good if we listed out all the files that the compiler possibly looks for to prevent further confusion.

Also, the sentence "Because these sub-modules don't have their own....." seemed to indicate that only two files - src/english/greetings.rs and src/japanese/farewells.rs need to be created, but in actuality four files need to be created - src/{english,japanese}/{greetings/farewells}.rs. This commit fixes this too.

Copy link
Member

Choose a reason for hiding this comment

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

Oh! Sorry about this, I misunderstood. I see now.

@steveklabnik
Copy link
Member

Thank you! I like the lifetimes changes, nice catch. I don't understand most of the changes in crates and modules, though.

Oh, it also looks like your git email isn't the same as your github one, you may want to change that as well? 😄

@steveklabnik
Copy link
Member

@varunvats I am ready to commit this, but can you rebase it to remove that merge commit, please?

Varun Vats added 2 commits April 5, 2016 12:09
1. In the English/Japanese phrases example in the "Multiple File
Crates" section of the "Crates and Modules" chapter, there are a total
of 8 module files that Rust looks for, while only four were
listed. This commit lists all 8 explicitly.
2. Title case fix.
@varunvats
Copy link
Author

@steveklabnik I was able to get rid of the merge commit by resetting and rebasing my docs-fix branch over rust-lang:master. This did change the IDs of my original commits, but I don't think that should cause any trouble since no one possibly cloned my docs-fix branch. Let me know if this would cause any issues with the PR.

@steveklabnik
Copy link
Member

Yup, that's absolutely fine! That's how it should work 😄

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Apr 5, 2016

📌 Commit a7d15ce has been approved by steveklabnik

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 6, 2016
Minor doc fixes in "Crates and Modules" and "Lifetimes" chapters

These commits fix a couple of (minor) issues in the _Crates and Modules_ and the _Lifetimes_ chapters of the book.

r? @steveklabnik
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016
Minor doc fixes in "Crates and Modules" and "Lifetimes" chapters

These commits fix a couple of (minor) issues in the _Crates and Modules_ and the _Lifetimes_ chapters of the book.

r? @steveklabnik
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016
Minor doc fixes in "Crates and Modules" and "Lifetimes" chapters

These commits fix a couple of (minor) issues in the _Crates and Modules_ and the _Lifetimes_ chapters of the book.

r? @steveklabnik
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Apr 6, 2016
Minor doc fixes in "Crates and Modules" and "Lifetimes" chapters

These commits fix a couple of (minor) issues in the _Crates and Modules_ and the _Lifetimes_ chapters of the book.

r? @steveklabnik
bors added a commit that referenced this pull request Apr 6, 2016
Rollup of 12 pull requests

- Successful merges: #31762, #32538, #32634, #32668, #32679, #32691, #32724, #32727, #32744, #32761, #32766, #32774
- Failed merges:
@bors bors merged commit a7d15ce into rust-lang:master Apr 6, 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.

4 participants