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

Add workspace root to metadata command. #4938

Merged
merged 1 commit into from
Jan 14, 2018
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jan 13, 2018

Fixes #4933

@alexcrichton, you mentioned using "workspace_manifest", but the Workspace.root function already strips off Cargo.toml. It would be easy to append it back, though I'm uncertain if that's really necessary since I think for most use cases it will just need to be stripped off again. Also, I feel like it might be confusing for non-workspace packages since workspace_manifest would be the same as the package manifest_path (and in that case it isn't really a workspace manifest). I can easily change it, just let me know.

@rust-highfive
Copy link

r? @alexcrichton

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

@alexcrichton
Copy link
Member

Nah looks good to me, thanks @ehuss!

@bors: r+

@bors
Copy link
Collaborator

bors commented Jan 13, 2018

📌 Commit fef215d has been approved by alexcrichton

@alexcrichton
Copy link
Member

@ehuss want to send this to the rust-1.24.0 branch as well for the backport?

@bors
Copy link
Collaborator

bors commented Jan 13, 2018

⌛ Testing commit fef215d with merge 6a8eb71...

bors added a commit that referenced this pull request Jan 13, 2018
Add workspace root to metadata command.

Fixes #4933

@alexcrichton, you mentioned using `"workspace_manifest"`, but the `Workspace.root` function already strips off `Cargo.toml`.  It would be easy to append it back, though I'm uncertain if that's really necessary since I think for most use cases it will just need to be stripped off again.  Also, I feel like it might be confusing for non-workspace packages since `workspace_manifest` would be the same as the package `manifest_path` (and in that case it isn't really a workspace manifest).  I can easily change it, just let me know.
@bors
Copy link
Collaborator

bors commented Jan 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 6a8eb71 to master...

@bors bors merged commit fef215d into rust-lang:master Jan 14, 2018
bors added a commit that referenced this pull request Jan 14, 2018
Add workspace root to metadata command.

Fixes #4933

Merge of #4938 for rust 1.24 beta.

@alexcrichton I'm uncertain about the process for merging in beta.  I'm guessing after this I just need to open a PR on the rust beta branch to update the cargo submodule?
@ehuss ehuss added this to the 1.25.0 milestone Feb 6, 2022
@ehuss ehuss modified the milestones: 1.25.0, 1.24.0 Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to detect workspace root?
4 participants