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

[doc]:fix project to package #13820

Closed
wants to merge 2 commits into from
Closed

Conversation

heisen-li
Copy link
Contributor

What does this PR try to resolve?

As [project] is being phased out, the filename should be consistent with the title of the file content.

@rustbot
Copy link
Collaborator

rustbot commented Apr 28, 2024

r? @epage

rustbot has assigned @epage.
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

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 28, 2024
@heisen-li heisen-li marked this pull request as draft April 28, 2024 12:15
@heisen-li
Copy link
Contributor Author

I'm going to check to see if the link jump is affected and set it to draft for now.

@heisen-li heisen-li marked this pull request as ready for review April 29, 2024 02:39
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

I forgot how things actually work, but we need to ensure that every old link won't be dead and will redirect to the new one. Should check both page and fragment redirections.

@heisen-li
Copy link
Contributor Author

I forgot how things actually work, but we need to ensure that every old link won't be dead and will redirect to the new one. Should check both page and fragment redirections.

I'm just talking about my own knowledge that there is a check for document linking in CI, but I don't know if it includes a check for fragment redirection.

https://raw.githubusercontent.com/rust-lang/rust/master/src/tools/linkchecker/linkcheck.sh

@weihanglo
Copy link
Member

We need to make sure the redirection work. For example, https://doc.rust-lang.org/nightly/cargo/guide/project-layout.html must redirect to https://doc.rust-lang.org/nightly/cargo/guide/package-layout.html.

@weihanglo
Copy link
Member

Speaking of that. This only affects URLs. It is not really a thing user preceive, so might not be worth having this churn.

@weihanglo
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2024
@heisen-li
Copy link
Contributor Author

Speaking of that. This only affects URLs. It is not really a thing user preceive, so might not be worth having this churn.

Of course, this is not relevant to the big picture. I was just browsing the article and noticed this.

The content of the URL doesn't match the content of the document, I just think it should be kept consistent. If this is difficult or otherwise, then feel free to turn it off.

Thank you. @weihanglo

@weihanglo
Copy link
Member

mdbook supports [output.html.redirect], though it doesn't seem to support fragment jumps after redirections. We then need to add additional JavaScript like this:

<script>
(function() {
var fragments = {
"#the-project-layout": "../guide/project-layout.html",
"#examples": "cargo-targets.html#examples",
"#tests": "cargo-targets.html#tests",
"#integration-tests": "cargo-targets.html#integration-tests",
"#configuring-a-target": "cargo-targets.html#configuring-a-target",
"#target-auto-discovery": "cargo-targets.html#target-auto-discovery",
"#the-required-features-field-optional": "cargo-targets.html#the-required-features-field",
"#building-dynamic-or-static-libraries": "cargo-targets.html#the-crate-type-field",
"#the-workspace-section": "workspaces.html#the-workspace-section",
"#virtual-workspace": "workspaces.html",
"#package-selection": "workspaces.html#package-selection",
"#the-features-section": "features.html#the-features-section",
"#rules": "features.html",
"#usage-in-end-products": "features.html",
"#usage-in-packages": "features.html",
"#the-patch-section": "overriding-dependencies.html#the-patch-section",
"#using-patch-with-multiple-versions": "overriding-dependencies.html#using-patch-with-multiple-versions",
"#the-replace-section": "overriding-dependencies.html#the-replace-section",
"#package-metadata": "manifest.html#the-package-section",
"#the-authors-field-optional": "manifest.html#the-authors-field",
"#the-edition-field-optional": "manifest.html#the-edition-field",
"#the-documentation-field-optional": "manifest.html#the-documentation-field",
"#the-workspace--field-optional": "manifest.html#the-workspace-field",
"#package-build": "manifest.html#the-build-field",
"#the-build-field-optional": "manifest.html#the-build-field",
"#the-links-field-optional": "manifest.html#the-links-field",
"#the-exclude-and-include-fields-optional": "manifest.html#the-exclude-and-include-fields",
"#the-publish--field-optional": "manifest.html#the-publish-field",
"#the-metadata-table-optional": "manifest.html#the-metadata-table",
};
var target = fragments[window.location.hash];
if (target) {
var url = window.location.toString();
var base = url.substring(0, url.lastIndexOf('/'));
window.location.replace(base + "/" + target);
}
})();
</script>

I feel like there are more works with less consistency gain, so lean toward closing. What do you think?

@heisen-li
Copy link
Contributor Author

Ok, no problem there. In the future, it would be nice to be able to check the validity of the link jump in the CI.

@heisen-li heisen-li closed this May 6, 2024
@heisen-li heisen-li deleted the project branch May 6, 2024 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants