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

Verify documentation links on travis #138

Closed
wants to merge 3 commits into from
Closed

Verify documentation links on travis #138

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 2, 2019

Fixes #82

@ghost
Copy link
Author

ghost commented Sep 2, 2019

@stjepang when you have a moment would you please take a look at this and let me know if it's what you had in mind for resolving #82? I think this should fix all the current errors produced by cargo deadlinks --check-http.

If it looks okay, I'll squash the commits and mark it ready for review.

One thing that will probably need to be changed is the installation step for cargo-deadlinks. Should I just create a ci/install-cargo-deadlinks.sh and follow the same template as ci/install-mdbook.sh?


cfg_if! {
Copy link

Choose a reason for hiding this comment

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

Let's also delete this section.

Copy link
Author

Choose a reason for hiding this comment

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

If I delete these doc shims in src/os/unix/fs.rs, then I start to get errors like this:

Found invalid urls in async-std/target/doc/async_std/os/unix/fs/index.html:
        Linked file at path async-std/target/doc/std/fs/struct.OpenOptions.html does not exist!

I tried to trace back to where all of these are being referenced but wasn't successful. Do you still want to try and remove them?

@ghost
Copy link

ghost commented Sep 4, 2019

One thing that will probably need to be changed is the installation step for cargo-deadlinks. Should I just create a ci/install-cargo-deadlinks.sh and follow the same template as ci/install-mdbook.sh?

Yes, that sounds reasonable to me!

@ghost
Copy link
Author

ghost commented Sep 7, 2019

@stjepang Thanks for the feedback!

Responding to a few specific points you raised:

  1. I've been pinning the toolchain since I work on Windows sometimes and the build is (was?) broken on current nightly.

  2. The trait@Send links do indeed work and they are currently pointed to the docs upstream.

  3. I've been unable to remove some of the shims in os/unix/fs.rs; see comments above.

Finally, installing cargo-deadlinks following the same template as ci/install-mdbook.sh isn't going to work. The reason is because the script at https://japaric.github.io/trust/install.sh has the release download url hard-coded for a naming format the deadlinks/cargo-deadlinks repo doesn't use.

Given that, I'm not sure how you want to handle the installation. Thoughts? (cc @jamesmunns)

Otherwise, I've made the requested changes.

@ghost ghost self-requested a review September 7, 2019 00:17
@ghost ghost marked this pull request as ready for review September 7, 2019 00:28
@yoshuawuyts yoshuawuyts added the documentation Improvements or additions to documentation label Sep 18, 2019
@ghost ghost mentioned this pull request Sep 29, 2019
@ghost
Copy link
Author

ghost commented Oct 20, 2019

Going to close this. If someone would like to resume the work, feel free to ping me.

@ghost ghost closed this Oct 20, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify documentation links on Travis
2 participants