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

Don't add the new package to workspace.members if there is no existing workspace in Cargo.toml. #13391

Merged
merged 2 commits into from
Feb 6, 2024

Conversation

linyihai
Copy link
Contributor

@linyihai linyihai commented Feb 4, 2024

What does this PR try to resolve?

Fixed #13345

If the current package has no workspace table in Cargo.toml, then if you run cargo add foo, don't create the workspace inline item and don't add foo into the workspace.members.

How should we test and review this PR?

Reviewed by commit by commit.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2024

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added Command-new S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 4, 2024
@linyihai linyihai force-pushed the non_workspace_add_lib branch from 4474dc2 to 4d2c0c6 Compare February 4, 2024 12:32
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 for working on this!

@linyihai linyihai force-pushed the non_workspace_add_lib branch from 4d2c0c6 to 6cd8a3a Compare February 5, 2024 02:33
@linyihai linyihai changed the title [WIP] Don't add the new package to workspace.members if there is no existing workspace in Cargo.toml. Don't add the new package to workspace.members if there is no existing workspace in Cargo.toml. Feb 5, 2024
@linyihai linyihai marked this pull request as ready for review February 5, 2024 03:48
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 pretty good!

members.push(display_path);
if was_sorted {
members.sort_by(|lhs, rhs| lhs.as_str().cmp(&rhs.as_str()));
workspace_document["workspace"]["members"] = toml_edit::value(array);
Copy link
Member

Choose a reason for hiding this comment

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

nit: one less access

Suggested change
workspace_document["workspace"]["members"] = toml_edit::value(array);
workspace["members"] = toml_edit::value(array);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point.

@weihanglo
Copy link
Member

Thanks for the patch!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 6, 2024

📌 Commit 0391b14 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 Feb 6, 2024
@bors
Copy link
Contributor

bors commented Feb 6, 2024

⌛ Testing commit 0391b14 with merge bb9b25f...

@bors
Copy link
Contributor

bors commented Feb 6, 2024

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

@bors bors merged commit bb9b25f into rust-lang:master Feb 6, 2024
23 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Update cargo

14 commits in cdf84b69d0416c57ac9dc3459af80dfb4883d27a..ccc84ccec4b7340eb916aefda1cb3e2fe17d8e7b
2024-02-02 19:39:16 +0000 to 2024-02-07 15:37:49 +0000
- Relax a test to permit warnings to be emitted, too. (rust-lang/cargo#13415)
- test: disable lldb test as it requires privileges to run on macOS (rust-lang/cargo#13416)
- Update git2 (rust-lang/cargo#13412)
- fix: Switch more notes/warnings to lowercase (rust-lang/cargo#13410)
- Don't add the new package to workspace.members if there is no existing workspace in Cargo.toml. (rust-lang/cargo#13391)
- Remove build metadata from curl-sys version. (rust-lang/cargo#13401)
- Fix markdown line break in cargo-add (rust-lang/cargo#13400)
- Remove `package.documentation` from the “before publishing” list. (rust-lang/cargo#13398)
- chore(deps): update gix (rust-lang/cargo#13380)
- chore(deps): update compatible (rust-lang/cargo#13379)
- feat(update): Tell users when they are still behind (rust-lang/cargo#13372)
- docs(changelog): Slight cleanup (rust-lang/cargo#13396)
- Bump to 0.79.0; update changelog (rust-lang/cargo#13392)
- doc: `[package]` doesn't require `version` field (rust-lang/cargo#13390)

r? ghost
@rustbot rustbot added this to the 1.78.0 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-new 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.

cargo-new: workspace is added as an inline table
4 participants