Skip to content

Add docs.rs integration #676

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

Merged
merged 4 commits into from
May 2, 2017
Merged

Conversation

kureuil
Copy link
Contributor

@kureuil kureuil commented Apr 10, 2017

When a crate does not specify a documentation link, we query docs.rs to know
if the documentation for the requested version was successfully built. If
if is, we set the crate's documentation link to the corresponding docs.rs
page, if not nothing changes.

Fixes #419.

When a crate does not specify a documentation link, we query docs.rs to know
if the documentation for the requested version was successfully built. If
if is, we set the crate's documentation link to the corresponding docs.rs
page, if not nothing changes.
@carols10cents
Copy link
Member

I am so excited about this!!! I've been waiting for someone to take care of this ❤️ ❤️ ❤️ I'm sorry for letting this sit for a few days!!!

So I tried this out locally against production's API by running yarn run start:live, and the following things work great!!

I think there's still a problem with crates that only have unstable versions on their crate page-- sassers is one of those, and I'm seeing the version number be undefined in the docs.rs request. I don't think that's a problem with your code though, I think that's basically a new symptom of #654/#510 :( So don't worry about that, and I'll try to fix that problem soon.

One question I have, though, is whether we should be doing this on crate list pages, like search result pages, /crates, category lists, keyword lists, etc since we have the documentation links there now too. I think it would be useful, and I hope doing a bunch of ajax requests to docs.rs on loads of those pages won't take too long or DDoS docs.rs, but we'll find out?

What do you think about trying to add that too?

@onur
Copy link
Member

onur commented Apr 15, 2017

@carols10cents when I started adding builds.json endpoint to docs.rs we didn't have documentation links in crate list pages. I don't think it's a good idea to send multiple requests to docs.rs in this pages (docs.rs can handle thanks to rust but it will probably take too long for client).

I'll try to add another method to query multiple crates with only one request.

One more thing: please use: https://docs.rs/${crateName}/${crateVersion} link instead of https://docs.rs/${crateName}/${crateVersion}/${crateName}/. Last part of the link might be different than ${crateName}. https://docs.rs/calculator for example, using exp for lib.name and calculator for package.name.

@kureuil
Copy link
Contributor Author

kureuil commented Apr 15, 2017

So, I just pushed some changes to the branch:

  • The generated link to docs.rs now omits the lib.name (thanks @onur)
  • No more undefined version numbers on a crate page with only unstable versions (I didn't see the query fetching all of the crates' versions was asynchronous)

@carols10cents I also think it would be very useful to have the auto-link on every page where a crate is referenced but I agree with @onur's argument, it might not be the best experience for end users to have up to N requests going to docs.rs for documentation links. Maybe we could fetch the build status in the backend and have the API return the docs.rs link if the crate did not specified a documentation link. This could also allow for more aggressive caching of the documentation link.
BTW, shouldn't the documentation link be stored in the Version model ? Why isn't the crate metadata (except for features) versionned ?

@carols10cents
Copy link
Member

Thanks for the updates! Taking a look at the changes now. It might be a bit confusing why a documentation link appears sometimes when you click into a crate's page, but we can add the docs.rs link to lists of crates when either docs.rs has an endpoint to query multiple crates at once or we add caching to crates.io's backend.

BTW, shouldn't the documentation link be stored in the Version model ? Why isn't the crate metadata (except for features) versionned ?

I don't know exactly :) I could see a reason for the documentation links to be associated with a version, but I don't think anyone was hosting anything other than their most recent documentation when crates.io was created. Links like the homepage and repo either won't change or if they change, the old ones will be inaccessible, so there isn't really much reason to store links per version. Same with description, authors, README, LICENSE, etc.

@carols10cents
Copy link
Member

carols10cents commented Apr 19, 2017

Something's still not quite right navigating around versions and getting doc links for versions, and it might be due to our misuse of ember in general, i'm not sure... here's how to reproduce what I'm seeing:

Let me know if you have any questions or if this doesn't make sense or if you'd like help investigating why this is happening :)

@kureuil
Copy link
Contributor Author

kureuil commented Apr 21, 2017

Good catch on the version change! I'll investigate this next week once I'm back from vacation.

As we edit the live model, the fetchCrateDocumentation was not being called as the documentation property of the
model was not undefined annymore.
Bonus feature: Any crate that has a docs.rs link as documentation automatically gets versioning on it
@kureuil
Copy link
Contributor Author

kureuil commented May 1, 2017

Finally took time to resolve this last issue. It should now work properly :)

Bonus feature: Any crate that has a docs.rs link as documentation automatically gets versioning on it (i.e: If I navigate to the lazy_static crate page and change version, the documentation is updated for the current version, because the crate specified https://docs.rs/crate/lazy_static as its documentation link)

Copy link
Member

@carols10cents carols10cents left a comment

Choose a reason for hiding this comment

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

WOOOO!!! This works great!!! Thank you thank you thank you!! ❤️

@carols10cents carols10cents merged commit 2217dd5 into rust-lang:master May 2, 2017
@kureuil
Copy link
Contributor Author

kureuil commented May 2, 2017

I'll take care of updating the cargo docs in the coming days :)

@kureuil kureuil deleted the docs-rs-integration branch May 2, 2017 20:03
bors added a commit to rust-lang/cargo that referenced this pull request May 7, 2017
Document new behavior of crates.io with the documentation field of the manifest

[crates.io](https://crates.io/) now automatically links a crate with its [docs.rs](https://docs.rs/) page if no documentation link was specified in the manifest.

See rust-lang/crates.io#676
@jplatte
Copy link

jplatte commented Jun 16, 2017

According to http://doc.crates.io/manifest.html#the-documentation-field-optional this should be in effect, right? I published my first crate today, but after about an hour I still don't see a documentation link on the crates.io page. docs.rs seems to have built the documentation just fine though.

@carols10cents
Copy link
Member

@jplatte I'm seeing the documentation link on libproxy's page:

screen shot 2017-06-17 at 12 02 32 pm

What browser are you using? Can you open developer tools to the network tab, reload the page, and see if there's a request to docs.rs as shown in this screenshot, and if that request succeeded or not?

Are you running any plugins in your browser that might be blocking the request, such as a privacy tool like EFF's privacy badger, an adblocker, anything like that?

@jplatte
Copy link

jplatte commented Jun 17, 2017

Ohhh, yeah my browser was blocking the request... I forgot that most of the crates.io interface is built on the client side.

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.

4 participants