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

To fix 03-setup/Readme.md references to next section links #131

Merged
merged 1 commit into from
Oct 31, 2018
Merged

To fix 03-setup/Readme.md references to next section links #131

merged 1 commit into from
Oct 31, 2018

Conversation

ChouzArt
Copy link
Contributor

As seen on #123

@ChouzArt ChouzArt requested a review from a team as a code owner October 18, 2018 22:15
@ChouzArt ChouzArt changed the title Fixed 03-setup/Readme.md references to next chapters Fixed 03-setup/Readme.md references to next section links Oct 18, 2018
@ChouzArt ChouzArt changed the title Fixed 03-setup/Readme.md references to next section links To fix 03-setup/Readme.md references to next section links Oct 18, 2018
Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Oct 18, 2018
131: To fix 03-setup/Readme.md references to next section links r=therealprof a=ChouzArt

As seen on #123 

Co-authored-by: Carlos <gsolracchz@gmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 18, 2018

Build failed

@therealprof
Copy link
Contributor

@rust-embedded/resources The markdown stuff is acting all funny. The absolute paths are different than what they're supposed to be and the relative paths don't work. Any ideas?

@ChouzArt
Copy link
Contributor Author

ChouzArt commented Oct 19, 2018

This is related to rust-embedded/bookshelf#4.
Bookshelf version is broken, standalone show images.

It can be fixed doing this trick rust-embedded/book#38.

The images issue #123 can be solved inserting /discovery/ into the link.
Inserting /discovery/ for the next section links doesn't do the trick

EDIT:
But without inserting root paths tricks, it should work fine.
The issue is in mdbook "print.html" generator. Build fails because mdbook does not allocate correct relative links in print file.
rust-lang-nursery/mdBook#789

bors bot added a commit that referenced this pull request Oct 31, 2018
142: Attempt to fix broken image links in Discovery book r=therealprof a=adamgreen

This PR pools together PRs #130 and #131 from @ChouzArt and adds 2 more commits that I created to correct other failing links missed by the first 2 and to modify the Travis verification steps to skip the checking of print.html - @ChouzArt has previously pointed out that rust-lang/mdBook#789 causes mdbook to generate an incorrect print.html file when relative links are used in the book content. Skipping verification of print.html during Travis runs means that we will still have a bad image links in print.html but the main Discovery book pages will have working images again and they will show up here on GitHub as well. I realize that this isn't a perfect solution but I thought that people might find it better than the current situation.

The main reason I am creating this PR is to:
* See if it will pass Travis.
* Get people's feedback on this **half** solution.

If we decide to take this PR then there are a few other things that need to be done:
* Create a new issue to track the fact that we should revert this ignoring of print.html during link checking once the mdbook issue is resolved.
* Make a similar change to the [Bookself version of the Travis script](https://github.com/rust-embedded/bookshelf/blob/master/ci/script.sh).

I should also point out that running a `sed` script on print.html can fix most of the link issues that it contains so that the images will at least show up in a printout. Something like:
```console
cp book/print.html print.html
sed 's: src="\.\./assets/: src="assets/:g' <print.html >book/print.html
rm print.html
```

Co-authored-by: Carlos <gsolracchz@gmail.com>
Co-authored-by: Adam Green <adamgreen@users.noreply.github.com>
@bors bors bot merged commit 9e6de9b into rust-embedded:master Oct 31, 2018
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