-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix link and image reference definitions and remove unused ones #15113
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I would recommend splitting up this PR by change type, either into multiple commits or even multiple PRs so its easier to review and easier to change directions
src/doc/src/reference/manifest.md
Outdated
@@ -91,7 +91,7 @@ a keyword. [crates.io] imposes even more restrictions, such as: | |||
|
|||
### The `version` field | |||
|
|||
The `version` field is formatted according to the [SemVer] specification: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the next paragraph, there is a inline link definition: [Semantic Versioning](https://semver.org/)
. It is the same as [SemVer]. Why not unify them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo if we consolidated, it would be to SemVer
.
Structurally, the way it is written works. It is describing a specific technical point and the next paragraph goes into greater detail, including in the name of the semantics / spec.
Another way this could be written is on first use of SemVer
we add (Semantic Versioning)
after. However, I think the meaning of SemVer
is fairly well understood and the link can clarify for anyone who doesn't. We use SemVer
throughout the docs and there is no single entry point for adding that parenthetical, so we'd either do it everywhere or nowhere.
Do you suggest one source file modification (only one kind of issue: fix link and image reference definition) per commit? |
Yes |
Got it. Thanks! I'll do it. |
6e841a9
to
82e9bd4
Compare
…issue for environment-variables.md
82e9bd4
to
72b069e
Compare
@epage This PR has been updated. One source file modification per commit. |
@@ -379,7 +378,6 @@ autobins = false | |||
[`cargo test`]: ../commands/cargo-test.md | |||
[cfg-test]: ../../reference/conditional-compilation.html#test | |||
[crate types]: ../../reference/linkage.html | |||
[crates.io]: https://crates.io/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't really follow the conversation, but it seems to me that the “Remove an unused link reference definition” changes could be combined into one commit, to avoid excessive commits. Anyway, it's up to you both to decide what’s better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Squash-and-merge is disabled in this repository (and in most repos in rust-lang org).
- To myself, commit messages are not only for one-time review, but also for future bisects and reverts. This random article has explained it well
- Again, I was just fly-by and wasn't really close enough to this PR review. I'll leave this to you and Ed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, its taken me some time to get back to this.
I think there was a misunderstanding. My intent wasn't literally one commit per source file but to fix one type of issue per commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole PR is about one issue, as mentioned in the title:
Fix link and image reference definitions and remove unused link and image reference definitions.
The Cargo Book source files, I mean the markdown files, have a lot of style issues.
I'm trying to fix them one by one. This PR is the first one: Fix link and image reference definitions and remove unused ones.
Issues: #15120