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

internal: extend check.overrideCommand and buildScripts.overrideCommand docs #15492

Merged
merged 3 commits into from Sep 22, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 21, 2023

Extend check.overrideCommand and buildScripts.overrideCommand docs regarding invocation strategy and location.

However something still seems a bit odd -- the docs for invocationStrategy/invocationLocation talk about "workspaces", but the setting that controls which workspaces are considered is called linkedProjects. Is a project the same as a workspace here or is there some subtle difference?

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2023
@Veykril
Copy link
Member

Veykril commented Aug 21, 2023

They are the same in this regard. The terms are kind of confusing, projects, workspaces, cargo workspaces, vscode workspaces. linkedProjects is named as such because it was initially meant for pointing to the rust-project.json format, but you can also point it to cargo manifests.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 21, 2023

To make things worse, the "root" InvocationLocation is documented as

The command will be executed in the project root.

So this uses "project" in a way very different from linkedProjects...

I pushed a change which should help elsewhere, but I don't have a good idea for what to do about this one.

@Veykril
Copy link
Member

Veykril commented Aug 21, 2023

We should probably define some terms and use them appropriately in the docs, as sometimes we do want to talk about the entire opened directory opposed to contained cargo workspace within. I can put that on my homework list

@Veykril
Copy link
Member

Veykril commented Sep 22, 2023

Damn, now i regret the invocationlocation config option being name workspace.... project would've been more appropriate I think

@Veykril
Copy link
Member

Veykril commented Sep 22, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

📌 Commit ba7f2bf has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

⌛ Testing commit ba7f2bf with merge 2b580a1...

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 2b580a1 to master...

@bors bors merged commit 2b580a1 into rust-lang:master Sep 22, 2023
2 of 10 checks passed
@lnicola lnicola changed the title extend check.overrideCommand and buildScripts.overrideCommand docs internal: extend check.overrideCommand and buildScripts.overrideCommand docs Sep 24, 2023
@RalfJung RalfJung deleted the invocation branch March 29, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants