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.

Sign up for GitHub

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

Jump to bottom

docs: clarify the use of default branch instead of main by default #12251

Merged
merged 2 commits into from
Jun 13, 2023
Merged

docs: clarify the use of default branch instead of main by default #12251

merged 2 commits into from
Jun 13, 2023

Conversation

c-git
Copy link
Contributor

@c-git c-git commented Jun 10, 2023

Update specifying-dependencies.md to show not specifying the branch for git doesn't always use the main branch.

@c-git
Record results from testing
8a1d6fd 
Update specifying-dependencies.md to show not specifying the branch for git doesn't always use the main branch.
@rustbot
Copy link
Collaborator

rustbot commented Jun 10, 2023

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 Jun 10, 2023
@weihanglo weihanglo changed the title Record results from testing docs: clarify the use of default branch instead of main by default Jun 12, 2023
weihanglo
weihanglo reviewed Jun 12, 2023
View reviewed changes
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.

Thanks and pardon my editing the PR title.

src/doc/src/reference/specifying-dependencies.md Outdated
Comment on lines 143 to 144
we intend to use the latest commit on the Default ("base") branch branch
to build our package, which may not necessarily be the main branch.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the catch!

I feel like there is still a confusion in this patch. People may wonder what a capital "Default" branch is, and what it is different from base. Is base the correct technical term for the "default branch"?

Technically speaking, Cargo use HEAD (refs/remotes/origin/HEAD)1 to find the default branch.

I would suggest the following change, since the default branch looks like a term in use in git's own doc2:

Suggested change
we intend to use the latest commit on the Default ("base") branch branch
to build our package, which may not necessarily be the main branch.
we intend to use the latest commit on the default branch to build our package

Footnotes

  1. https://github.com/rust-lang/cargo/blob/928b9561c16e73fca6206b42e4c464b80b0e9221/src/cargo/sources/git/utils.rs#L959-L961 ↩

  2. Git configuration init.defaultBranch ↩

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took the terms from GitHub's settings page. I don't actually know. I used the capital because they had it as capital, I don't feel strongly about it.IMO I think what you said is better than both. Say how it will be determined. Rather than trying to name it. As long as it is clear that it's not always main. Happy to make the edits if you prefer when I get home but you're free to edit as you see fit as well.

@c-git
Copy link
Contributor Author

c-git commented Jun 13, 2023

@weihanglo I'm sorry, not sure if I'm the one who will make the change of if you will update as you see fit.

@weihanglo
Copy link
Member

Personally I prefer my own suggestion, which only changes one word without introducing too many new concepts. That said, if you have a better solution putting a proper amount of information without leaking too many implementation details, I am happy to merge.

Both ways can be made by you I believe so :)

@c-git
Copy link
Contributor Author

c-git commented Jun 13, 2023

I updated it with the change you requested. If I missed anything please let me know. Actually looking at your comments again, I'm not sure if I captured what you said properly, please let me know.

weihanglo
weihanglo approved these changes Jun 13, 2023
View reviewed changes
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.

Looks good to me. Thanks!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2023

📌 Commit 89ca816 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 Jun 13, 2023
@bors
Copy link
Contributor

bors commented Jun 13, 2023

⌛ Testing commit 89ca816 with merge 000868e...

@bors
Copy link
Contributor

bors commented Jun 13, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 000868e to master...

@bors bors merged commit 000868e into rust-lang:master Jun 13, 2023
@weihanglo weihanglo mentioned this pull request Jun 14, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2023
@bors
Auto merge of rust-lang#112601 - weihanglo:update-cargo, r=weihanglo
5a65be8 
Update cargo

11 commits in 49b6d9e179a91cf7645142541c9563443f64bf2b..0c14026aa84ee2ec4c67460c0a18abc8519ca6b2
2023-06-09 17:21:19 +0000 to 2023-06-14 18:43:05 +0000
- fix(embedded): Don't append hash to bin names (rust-lang/cargo#12269)
- Fix version requirement example in Dependency Resolution, SemVer compatibility section (rust-lang/cargo#12267)
- Update triagebot links. (rust-lang/cargo#12265)
- Show a better error when container tests fail. (rust-lang/cargo#12264)
- chore: update dependencies (rust-lang/cargo#12261)
- refactor(embedded) (rust-lang/cargo#12262)
- docs: clarify the use of `default` branch instead of `main` by default (rust-lang/cargo#12251)
- docs: update changelog for 1.71 backport and 1.72 (rust-lang/cargo#12256)
- feat: Initial support for single-file packages (rust-lang/cargo#12245)
- test(z-flags): Verify `-Z` flags list is sorted (rust-lang/cargo#12224)
- refactor: registry data kinds cleanup (rust-lang/cargo#12248)

---

This commit also update LICENSE exceptions, as Cargo introduced a newer version of `dunce` and `blake3` as dependencies.

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 16, 2023
@bors
Auto merge of #112601 - weihanglo:update-cargo, r=weihanglo
4f1474a 
Update cargo

11 commits in 49b6d9e179a91cf7645142541c9563443f64bf2b..0c14026aa84ee2ec4c67460c0a18abc8519ca6b2
2023-06-09 17:21:19 +0000 to 2023-06-14 18:43:05 +0000
- fix(embedded): Don't append hash to bin names (rust-lang/cargo#12269)
- Fix version requirement example in Dependency Resolution, SemVer compatibility section (rust-lang/cargo#12267)
- Update triagebot links. (rust-lang/cargo#12265)
- Show a better error when container tests fail. (rust-lang/cargo#12264)
- chore: update dependencies (rust-lang/cargo#12261)
- refactor(embedded) (rust-lang/cargo#12262)
- docs: clarify the use of `default` branch instead of `main` by default (rust-lang/cargo#12251)
- docs: update changelog for 1.71 backport and 1.72 (rust-lang/cargo#12256)
- feat: Initial support for single-file packages (rust-lang/cargo#12245)
- test(z-flags): Verify `-Z` flags list is sorted (rust-lang/cargo#12224)
- refactor: registry data kinds cleanup (rust-lang/cargo#12248)

---

This commit also update LICENSE exceptions, as Cargo introduced a newer version of `dunce` and `blake3` as dependencies.

r? `@ghost`
@ehuss ehuss added this to the 1.72.0 milestone Jun 22, 2023
@weihanglo weihanglo mentioned this pull request Aug 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

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
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.
5 participants
@c-git @rustbot @weihanglo @bors @ehuss