Skip to content

Conversation

@tisonkun
Copy link
Contributor

Corresponding to f84f377 and as a side effect, make it work on GitHub.

Back link to #66888.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@ehuss
Copy link
Contributor

ehuss commented Nov 18, 2025

Can you say more about why we would make this change?
The link at https://doc.rust-lang.org/rust.html seems to work fine for me.
I would echo the comment at #66888 (comment) that if you are viewing files on GitHub, they are not expected to work there.

@tisonkun
Copy link
Contributor Author

Can you say more about why we would make this change? The link at https://doc.rust-lang.org/rust.html seems to work fine for me. I would echo the comment at #66888 (comment) that if you are viewing files on GitHub, they are not expected to work there.

This follows what f84f377 does that was accepted and it's a net win.

@ehuss
Copy link
Contributor

ehuss commented Nov 18, 2025

I don't think that quite answers the question. Why is f84f377 a net win? The relative links we use are designed to handle the different deployment scenarios that we use. There are thousands of relative links that when viewed in the GitHub source browser don't work. These source files aren't designed to work that way.

@tisonkun
Copy link
Contributor Author

OK. If you don't like this kind of change, I'll close this PR.

It's not worth the debt on whether it's a net win. From my point of view, here is a patch that you can make use of, no matter whether there is another.

If you have spare time, you may ask the reviewer of #87031 why he/she accepted that PR.

This is anyway a chore task I spot when going through the project. If the maintainer doesn't like it, I'm OK not to do.

@tisonkun tisonkun closed this Nov 18, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 18, 2025
@tisonkun tisonkun deleted the patch-1 branch November 18, 2025 05:24
@weihanglo
Copy link
Member

@tisonkun

Relative paths are essential here because it should redirect to the same version/channel. Also, the documentations are also bundled in Rust distribution tarballs, and we want to esure link redirections are self-contained (better for offline reading experience). I believe that is the "different deployment" ehuss was talking about.

f84f377 was kinda wrong and broke the link. Like, the link in https://doc.rust-lang.org/nightly/reference.html (mind "/nightly") jumps to https://doc.rust-lang.org/stable/reference/introduction.html (mind "/stable") and doesn't stay in /nightly/reference/introduction.html.

We might want to fix/revert f84f377 instead I guess, cc @ehuss.

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