Skip to content

Minor documentation fixes #22027

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

Merged
merged 3 commits into from
Feb 18, 2015
Merged

Minor documentation fixes #22027

merged 3 commits into from
Feb 18, 2015

Conversation

iblech
Copy link
Contributor

@iblech iblech commented Feb 6, 2015

The first commit adds a short note which I believe will reduce worries in people who work with closures very often and read the Rust book for their first time.

The second commit consists solely of tiny typo fixes. In some cases, I changed "logical" quotations like

She said, "I like programming".

to

She said, "I like programming."

because the latter seems to be the prevalent style in the book.

Without such a clarification, people who know and love closures (for instance
programmers with a Haskell background) might fear that types would have to
be declared in closures and that therefore using closures would be much more
unwieldy.
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (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.

@nikomatsakis
Copy link
Contributor

Thanks for the suggestion! I'm assigning to @steveklabnik -- he may want to tweak the wording; I feel like this is a part of the book that gets encountered fairly early, if I'm not mistaken. Personally I'd probably change it to something like "(Note though that types do not have to be declared in closures.)"

@iblech
Copy link
Contributor Author

iblech commented Feb 6, 2015

Thanks for the quick reply! Indeed, I hit that part of the book early. @steveklabnik, of course you can feel free to change the wording and/or ask me to do it (as a further commit to this pull request).

@@ -15,7 +15,7 @@ comments":
// the "link" crate attribute is currently required for rustdoc, but normally
// isn't needed.
#![crate_id = "universe"]
#![crate_type="lib"]
#![crate_type = "lib"]
Copy link
Member

Choose a reason for hiding this comment

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

this change is unrelated and shouldn't be here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't want to create too much noise with trivial pull requests. Do you want me to open a new one for this issue?

(I don't have a clue about correct Rust coding style. But http://aturon.github.io/style/whitespace.html recommends it and I thought it would be good to have the official documentation align with it.)

Copy link
Member

Choose a reason for hiding this comment

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

it's all good! no PR is too small. That guideline is WIP, and I would say goes against most style stuff, so let's just leave this for now, since it's already here.

@steveklabnik
Copy link
Member

Thanks so much!

Yes, we haven't even mentioned closures yet, and we do mention this behavior quite specifically, so I would prefer to leave it off. Most of the other changes are wonderful, though I do have a few points that shouldn't have been changed.

@steveklabnik
Copy link
Member

@iblech : one week bump. Any interest in updating this PR?

@iblech
Copy link
Contributor Author

iblech commented Feb 16, 2015

Sorry for not returning to you earlier, I was busy with other stuff. Also thank you for the detailed comments!

Are you sure about the removal of the parenthetical remark "(For closures, i.e. unnamed functions, types do not have to be declared.)"? It could be softed down, like "(If you already know closures, i.e. unnamed functions, then note that types do not have to be declared in those.)" This way I think it won't confuse people unfamiliar with closures, while greatly relieving folks like Haskell programmers.

@steveklabnik
Copy link
Member

It's all good!

Yes. We haven't covered closures at all yet, and so introducing the concept without explanation is a distraction. Closures are something that's becoming more and more popular, but I've taken a lot of care to only introduce what's needed at the time that it's needed.

@iblech
Copy link
Contributor Author

iblech commented Feb 16, 2015

Okay, fine, I'll revert the addition of the remark then.

Could you please check my answer to your line note regarding src/doc/trpl/documentation.md? After this, I can add all the necessary commits/reverts to this pull request. :-)

@steveklabnik
Copy link
Member

sorry, which line note?

@iblech
Copy link
Contributor Author

iblech commented Feb 16, 2015

Meanwhile, you already did! It's now hidden, because of the new commit.

@steveklabnik
Copy link
Member

Ha! Yeah, so I think this is good to go. Thank you so much!

@steveklabnik
Copy link
Member

@bors: r+ 918d097 rollup

@iblech
Copy link
Contributor Author

iblech commented Feb 16, 2015

Great! Thanks for the assistance. :-)

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 17, 2015
The first commit adds a short note which I believe will reduce worries in people who work with closures very often and read the Rust book for their first time.

The second commit consists solely of tiny typo fixes. In some cases, I changed "logical" quotations like

    She said, "I like programming".

to

    She said, "I like programming."

because the latter seems to be the prevalent style in the book.
@huonw huonw merged commit 918d097 into rust-lang:master Feb 18, 2015
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.

5 participants