- Notifications
You must be signed in to change notification settings - Fork13.9k
Add typo suggestion for a misspelt Cargo environment variable#148559
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
rustbot has assigned@fee1-dead. Use |
This comment has been minimized.
This comment has been minimized.
48c2fbb to1f8b6daCompare| constKNOWN_CARGO_VARS:&[&str] =&[ | ||
| // List of known Cargo environment variables that are set for crates (not build scripts, OUT_DIR etc). | ||
| // See: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Discussion (non-blocking): I have two reservations about making this suggestion:
- Wouldn't this depend on the specific cargo version?
- Basically, what I was trying to get at, is thatin principle I feel like
rustcshould not make assumptions aboutcargo(that is,rustcshould not "know" aboutcargo's existence), becausecargoisn't the only way to invokerustc.
I guess I could be convinced by "well, most usersdo build withcargo", so don't consider this a hard-blocking concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
- I'm not sure, there is a similar list for this in
rust-analyzer:https://github.com/chenyukang/rust/blob/702bf000a0b5062d8b5ced717a26d70919df6ba0/src/tools/rust-analyzer/crates/project-model/src/env.rs#L9-L49 , I guess there is a better and general way to keep it synced. - I agree
rustcshould not knowing details about cargo, I think it's ok since we need to check the prefix withCARGO_of env:
https://github.com/rust-lang/rust/pull/148559/files#diff-0eaa1095d59f2f3d9c015005d15fd8f7cda13121a8404d77b2955fc5fcd40449R213, and it's a note but not code suggestions, it's maybe incorrect for rare scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, seems alright, was just curious to hear what you think.
Uh oh!
There was an error while loading.Please reload this page.
1f8b6da toad471f4Compare
Uh oh!
There was an error while loading.Please reload this page.
Fixes#148439