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

isogram, two-bucket: use second-edition links #382

Merged
merged 1 commit into from
Nov 4, 2017
Merged

isogram, two-bucket: use second-edition links #382

merged 1 commit into from
Nov 4, 2017

Conversation

petertseng
Copy link
Member

We forgot to apply #368 to
#378 and
#375.

Copy link
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

There are some Markdown redundancies here, where we include lines which aren't rendered. I only specifically marked them for the isogram example, but the comments apply to both files.

I think we should decide whether to use inline or footnote-style links, and then stick to it.

@@ -42,7 +42,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
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the links are expressed both in footnote form and inline. I'm pretty sure this is redundant: if we have an inline link of the form [link_text](https://...), we don't need to include a footnote with the link's value. Contrariwise, if we want to use the footnote style, then the place where the link is actually used should look like [Modules][modules].

@@ -42,7 +42,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
[cargo]: 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.

This footnote isn't referenced at all, and is stripped out of the rendered document.

@petertseng
Copy link
Member Author

Correct on both counts, but not applicable to this PR.

@coriolinus
Copy link
Member

Just saw that these changes were applied by a script. Fair enough; there's no reason to add manual work to generated output. I withdraw my objection.

@petertseng
Copy link
Member Author

However, it is a true statement that those can be removed. I would simply remove them. Only question is whether https://doc.rust-lang.org/book/second-edition/ch14-00-more-about-cargo.html is needed. I took a < 5 second glance and made a snap decision of "no", but obviously a more reasoned decision should take priority over the five second decision.

@petertseng petertseng merged commit 16e7cf0 into exercism:master Nov 4, 2017
@petertseng petertseng deleted the edition branch November 4, 2017 21:46
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