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 links with correct reference to assets folder #130

Merged
merged 1 commit into from
Oct 31, 2018
Merged

To fix links with correct reference to assets folder #130

merged 1 commit into from
Oct 31, 2018

Conversation

ChouzArt
Copy link
Contributor

Cancelled a previous PR #121 in favor of this one. To make it cleaner using a new branch instead of my master.

#123 Also pointed to this issue.

@ChouzArt ChouzArt requested a review from a team as a code owner October 18, 2018 21:55
@ChouzArt ChouzArt changed the title Fix links with correct backward reference to assets Fix links with correct reference to assets folder Oct 18, 2018
@ChouzArt ChouzArt changed the title Fix links with correct reference to assets folder To fix links with correct reference to assets folder Oct 18, 2018
Copy link

@thejpster thejpster left a comment

Choose a reason for hiding this comment

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

In the absence of any other comments, this LGTM.

@thejpster
Copy link

bors r+

bors bot added a commit that referenced this pull request Oct 28, 2018
130: To fix links with correct reference to assets folder r=thejpster a=ChouzArt

Cancelled a previous PR #121 in favor of this one. To make it cleaner using a new branch instead of my master.

#123 Also pointed to this issue.

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

bors bot commented Oct 28, 2018

Build failed

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

2 participants