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

Updates all references from first to second edition of Rust book #368

Merged
merged 2 commits into from
Oct 28, 2017
Merged

Updates all references from first to second edition of Rust book #368

merged 2 commits into from
Oct 28, 2017

Conversation

nfiles
Copy link
Contributor

@nfiles nfiles commented Oct 21, 2017

References #356 (not sure if it closes it or not)

The only remaining references to the first edition are in these problems:

  • decimal
    • derive traits: topic isn't written yet (Appendix C)
  • parallel letter frequency
    • benchmark tests: I can't find any reference to this topic in the second edition
    • nightly rust: topic isn't written yet (Appendix D)

For reference, this is the search string I used to find the references: doc.rust-lang.org

And this is the script I used to update the most common first-edition references:

#! /bin/sh

# reference in the content
sed \
    -r 's|\[Crates and Modules\]\(https://doc.rust-lang.org/stable/book/crates-and-modules.html\)|\[Modules\]\(https://doc.rust-lang.org/book/second-edition/ch07-00-modules.html\)|' \
    -i exercises/*/README.md

# footnotes
sed \
    -r 's|\[crates-and-modules\]: http://doc.rust-lang.org/stable/book/crates-and-modules.html|\[modules\]: https://doc.rust-lang.org/book/second-edition/ch07-00-modules.html\n\[crates\]: https://doc.rust-lang.org/book/second-edition/ch14-00-more-about-cargo.html|' \
    -i exercises/*/README.md

@@ -27,4 +27,5 @@ The [exercism/rust](https://github.com/exercism/rust) repository on GitHub is th
If you want to know more about Exercism, take a look at the [contribution guide](https://github.com/exercism/docs/blob/master/contributing-to-language-tracks/README.md).

[help-page]: http://exercism.io/languages/rust
[crates-and-modules]: http://doc.rust-lang.org/stable/book/crates-and-modules.html
Copy link
Member

Choose a reason for hiding this comment

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

this is not on you, but I think this was unused. If it were used, I think I would have seen a link that looks like[Crates and modules][crates-and-modules]. We might want to look into that, and maybe a future PR can simply remove the [modules]: and [cargo]: if they are similarly not used. But I would keep what's been done in this PR, since it is just doing what it says it's doing (replace 1st ed with 2nd ed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I forgot how named links in markdown work. That makes sense.

@petertseng
Copy link
Member

I would like to say that this PR does close the linked issue since we have taken all the actions that we can at the current time. I wish there were a way to keep some record of things that we might want to check/take action on in the future. Perhaps an issue is still the right thing to do, just with a certain label.

@@ -66,7 +66,8 @@ The [exercism/rust](https://github.com/exercism/rust) repository on GitHub is th
If you want to know more about Exercism, take a look at the [contribution guide](https://github.com/exercism/docs/blob/master/contributing-to-language-tracks/README.md).

[help-page]: http://exercism.io/languages/rust
[crates-and-modules]: http://doc.rust-lang.org/stable/book/crates-and-modules.html
[modules]: https://doc.rust-lang.org/book/second-edition/ch07-00-modules.html
[crates]: https://doc.rust-lang.org/book/second-edition/ch14-00-more-about-cargo.html
Copy link
Member

Choose a reason for hiding this comment

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

differs from config/exercise-readme-insert.md. In particular, the name is crates here but cargo in config/exercise-readme-insert.md. I'm going to regenerate all READMEs to make them the same.

@petertseng
Copy link
Member

So, I hope you don't mind: To make this easier to review, I need to split this up into two commits.

One is just editing exercise-readme-insert.md and then regenerating all READMEs using it. That's all automatic and the only file that needs to be reviewed in that particular commit is exercise-readme-insert.md. Then the other commit is all the other files, and all files in that commit should get reviewed.

If this is not done I was having too hard of a time knowing which files I actually need to review.

@petertseng
Copy link
Member

Same as in #365 (comment) . Commit message is being changed from "Updates" to "Update".

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

Nice job. Don't worry about the comments that start with "not on you". Just two relevant things to deal with, I think.

docs/ABOUT.md Outdated

Rust core and the standard library contain a minimal set of functionality. Rustaceans are encouraged to add features, in the form of libraries called _crates_, to the language and then share them on [crates.io](https://crates.io/).

As a systems-level language, Rust is frequently used for building any tool where speed, performance and stability are paramount. The [Awesome Rust](https://github.com/kud1ing/awesome-rust) list collects examples of Rust projects, which include CLI tools, ORMs, operating systems and games. Regardless of what you build in Rust, it will be fast and memory safe!

The home page for Rust is [rust-lang.org](https://www.rust-lang.org/). Rust has excellent documentation at [rust-lang.org/documentation.html](https://www.rust-lang.org/documentation.html). Newcomers should start with "The Book" located at [doc.rust-lang.org/book/](https://doc.rust-lang.org/book/).
The home page for Rust is [rust-lang.org](https://www.rust-lang.org/). Rust has excellent documentation at [rust-lang.org/documentation.html](https://www.rust-lang.org/documentation.html). Newcomers should start with "The Book" located at [doc.rust-lang.org/book/](https://doc.rust-lang.org/book/second-edition/).
Copy link
Member

Choose a reason for hiding this comment

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

about doc.rust-lang.org/book/:

Previously, the link target was indicated by the link text, so if I were to copy the link text and paste it, I would reach the intended target. This is no longer true.

Is it important to keep true?

I don't care so if nobody else cares we will keep it as it currently is in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not consider that. I think it would make more sense for the two links to match, but it's not a big deal.

@@ -7,7 +7,7 @@ verify that all the pairs are matched and nested correctly.

Reading about these Rust topics may help you implement a solution.

- Lifetimes and Structs: https://doc.rust-lang.org/book/lifetimes.html#impl-blocks
- Lifetimes and Structs: https://doc.rust-lang.org/book/second-edition/ch10-03-lifetime-syntax.html#lifetime-annotations-in-struct-definitions
Copy link
Member

Choose a reason for hiding this comment

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

So, it looks to me like the link to https://doc.rust-lang.org/book/lifetimes.html#impl-blocks (which would now be reachable at https://doc.rust-lang.org/book/first-edition/lifetimes.html#impl-blocks) is meant to help with Brackets::from("[]"). In light of that, might I suggest that https://doc.rust-lang.org/book/second-edition/ch10-03-lifetime-syntax.html#lifetime-annotations-in-method-definitions looks more relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.


Note: It is [not idiomatic Rust to implement traits on on primitives](https://doc.rust-lang.org/book/traits.html#rules-for-implementing-traits). In this exercise we're showing something that you _can_ do, not something you _should_ do. If you find yourself implementing traits on primitives, perhaps you have a case of [Primitive Obsession](http://wiki.c2.com/?PrimitiveObsession).
Copy link
Member

Choose a reason for hiding this comment

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

not on you, so no need to fix in this PR (can in separate commit in this PR, or new PR): "on on"


Note: It is [not idiomatic Rust to implement traits on on primitives](https://doc.rust-lang.org/book/traits.html#rules-for-implementing-traits). In this exercise we're showing something that you _can_ do, not something you _should_ do. If you find yourself implementing traits on primitives, perhaps you have a case of [Primitive Obsession](http://wiki.c2.com/?PrimitiveObsession).
Note: It is [not idiomatic Rust to implement traits on on primitives](https://doc.rust-lang.org/book/second-edition/ch10-02-traits.html#implementing-a-trait-on-a-type). In this exercise we're showing something that you _can_ do, not something you _should_ do. If you find yourself implementing traits on primitives, perhaps you have a case of [Primitive Obsession](http://wiki.c2.com/?PrimitiveObsession).
Copy link
Member

Choose a reason for hiding this comment

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

not on you so no need to change this PR for this: I don't think either the first edition or the second edition link argues that you should not implement traits on primitives.

@nfiles
Copy link
Contributor Author

nfiles commented Oct 25, 2017

Splitting the huge commit into two separate ones is a good call.

I didn't notice that the README.md files were generated! That's awesome.

@petertseng
Copy link
Member

I didn't notice that the README.md files were generated!

In case anyone is reading this message and wants to generate the READMEs as well, until #354 receives approval so that it can be merged, the Rust track's READMEs are currently generated using a state that actually does not correspond to any exercism/problem-specifications commit; instead, a state that correctly reproduces the current Rust READMEs on master is https://github.com/petertseng/exercism-problem-specifications/tree/rust-readmes.

@petertseng
Copy link
Member

petertseng commented Oct 26, 2017

I just merged a new exercise, so, this PR should get rebased on master so that the README of the new exercise gets the generated change as well.

I can do that in a few hours. I sure don't want to add more work on your plate through no fault of yours.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

OK, that's all done. I'm out of the allotted time so I can't double check it, necessary before merging. Later.

@@ -7,7 +7,7 @@ verify that all the pairs are matched and nested correctly.

Reading about these Rust topics may help you implement a solution.

- Lifetimes and Structs: https://doc.rust-lang.org/book/lifetimes.html#impl-blocks
- Lifetimes and Structs: https://doc.rust-lang.org/book/second-edition/ch10-03-lifetime-syntax.html#lifetime-annotations-in-method-definitions
Copy link
Member

Choose a reason for hiding this comment

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

whoa there, forgot to regenerate, this should be in both hints and readme

Copy link
Member

Choose a reason for hiding this comment

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

Now it is.

@petertseng petertseng merged commit fb2aa17 into exercism:master Oct 28, 2017
@petertseng
Copy link
Member

Thanks for taking the time to update all these; fantastic work

@nfiles
Copy link
Contributor Author

nfiles commented Oct 30, 2017

No problem! Thanks for reviewing everything and directing me through this. It was a ton of work and is very appreciated. :)

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.

2 participants