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: Edits for git/path dependency sections #13341

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

rimutaka
Copy link
Contributor

@rimutaka rimutaka commented Jan 24, 2024

Fixes #9624

Summary

Made changes to [Specifying dependencies from git repositories] and Specifying path dependencies sections of the Reference.

  • minor rephrasing for readability
  • added more examples
  • added sub-headings
  • rearranged a few paragraphs for readability

Additional info

@weihanglo proposed a small change in this comment #9624 (comment).
It had been a while since I read that reference page, so I had a novice's point view and expanded the scope of the change to address the questions that I had while reading it.

The combination of git, path and version keys was specially confusing until I read the Multiple locations section.

If you think this is too much change for no gain I am happy to reverse the edits and only add the new info as per @weihanglo 's comment.

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) soon.

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@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 Jan 24, 2024
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.

As it is marked as draft, I didn't look into the code

  • minor rephrasing for readability
  • added more examples
  • added sub-headings
  • rearranged a few paragraphs for readability

It would be great if we can put these changes into their own commits, so it would be easier to land one of them separately while discussing others.

@rimutaka rimutaka changed the title Doc edits for git/path dependency sections #9624 [WIP] doc: Edits for git/path dependency sections #9624 Jan 30, 2024
@rimutaka rimutaka changed the title [WIP] doc: Edits for git/path dependency sections #9624 [WIP] doc: Edits for git/path dependency sections Jan 30, 2024
@rimutaka rimutaka marked this pull request as ready for review February 28, 2024 00:29
@rimutaka
Copy link
Contributor Author

@weihanglo , sorry, I could not separate the changes as you requested and still keep the document coherent and concise. Feel free to request rework or rollback for any of the parts. I probably create more work for you as a reviewer, so it's OK if you park this and maybe someone else can re-use it later.

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.

Thank you. Generally it looks better!

Still prefer making commit atomic, but it's not a hard blocker :)

src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
@weihanglo weihanglo changed the title [WIP] doc: Edits for git/path dependency sections doc: Edits for git/path dependency sections Feb 29, 2024
@weihanglo
Copy link
Member

According to this is marked as "ready", I went ahead and removed WIP from PR title and description :)

@rimutaka
Copy link
Contributor Author

@weihanglo , thanks a lot. I'll tidy it up as per your comments.

@rimutaka rimutaka changed the title doc: Edits for git/path dependency sections [WIP] doc: Edits for git/path dependency sections Mar 5, 2024
@rimutaka rimutaka force-pushed the git-path-doc-9624 branch from eb2b0e9 to 56e46a0 Compare March 5, 2024 01:13
@rimutaka rimutaka changed the title [WIP] doc: Edits for git/path dependency sections doc: Edits for git/path dependency sections Mar 5, 2024
@rimutaka
Copy link
Contributor Author

rimutaka commented Mar 5, 2024

@weihanglo , I think I applied all your changes except for toml formatting. Please, take a look when you have a moment.

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.

Could you tidy up your commit history? Either squash into one or whatever makes sense. There are some intermediate commits which is not ideal nor harder to track in the future.

src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
src/doc/src/reference/specifying-dependencies.md Outdated Show resolved Hide resolved
rimutaka added 2 commits March 6, 2024 08:03
* rephrasing for readability
* added more examples
* added sub-headings
* rearranged paragraphs for readability
@rimutaka rimutaka force-pushed the git-path-doc-9624 branch from 35c1c53 to 6b2c79c Compare March 5, 2024 19:57
@rimutaka
Copy link
Contributor Author

rimutaka commented Mar 5, 2024

@weihanglo , changes made as requested, commits are down to two.
Some of the things you picked up I should have noticed myself. Thank you for your patience with me.
Happy to make more changes if needed.

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.

Cool. Thanks for the writing! We shouldn't hold it back for any longer while. I am going to merge this.

If people have thoughts about this, always open for follow-up PRs!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 5, 2024

📌 Commit 6b2c79c has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2024
@bors
Copy link
Contributor

bors commented Mar 5, 2024

⌛ Testing commit 6b2c79c with merge 1c01c62...

@bors
Copy link
Contributor

bors commented Mar 5, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 1c01c62 to master...

@bors bors merged commit 1c01c62 into rust-lang:master Mar 5, 2024
21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
Update cargo

11 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..c3c417b85e01a1de1633317fa55e4f1a31e346d4
2024-03-01 22:57:35 +0000 to 2024-03-06 01:16:08 +0000
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2024
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35 +0000 to 2024-03-06 22:15:17 +0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
@rustbot rustbot added this to the 1.78.0 milestone Mar 7, 2024
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35 +0000 to 2024-03-06 22:15:17 +0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Update cargo

14 commits in f772ec0224d3755ce52ac5128a80319fb2eb45d0..a4c63fe5388beaa09e5f91196c86addab0a03580
2024-03-01 22:57:35 +0000 to 2024-03-06 22:15:17 +0000
- fix(cli): Skip tracing-chrome for platforms without 64bit atomics (rust-lang/cargo#13551)
- chore: downgrade to openssl v1.1.1 (again) (rust-lang/cargo#13550)
- fix(cli): Add traces to clarify where time is going (rust-lang/cargo#13545)
- fix(rustdoc-map): dedup `--extern-html-too-url` for same unit (rust-lang/cargo#13544)
- test: Add test for packaging a public dependency (rust-lang/cargo#13536)
- doc: Edits for git/path dependency sections (rust-lang/cargo#13341)
- feat(cli): Allow logging to chrome traces (rust-lang/cargo#13399)
- fix(log): Trace parameters to align with profile (rust-lang/cargo#13538)
- fix(toml): Don't warn on unset Edition if only 2015 is compatible (rust-lang/cargo#13533)
- fix(cli): Trace core cargo operations (rust-lang/cargo#13532)
- chore: update pulldown-cmark to 0.10.0 (rust-lang/cargo#13517)
- feat(add): Fallback to `rustc -v` when no MSRV is set (rust-lang/cargo#13516)
- chore(ci): Ensure lockfile is respected during MSRV testing (rust-lang/cargo#13523)
- feat: Use consistent colors when testing (rust-lang/cargo#13520)
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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Patch git dependency in a workspace or sub-directory
4 participants