-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Document CARGO_ALIAS_<name> #16521
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
Open
pnkfelix
wants to merge
1
commit into
rust-lang:master
Choose a base branch
from
pnkfelix:patch-1
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Document CARGO_ALIAS_<name> #16521
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is the case for most environment variables, including ones with substitution like this.
The environment variables we do talk about are either
bool(1and0)Is there something specific to
CARGO_ALIAS_*that we are needing to cover or are you feeling our approach to environment variables is lacking and could you describe why?FYI this might be better discussed in an issue. Multiple PRs may be created for the same issue which can split discussion and if we need to find past design discussions, we tend to look among Issues, not Prs.
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.
The other environment variables have their full names as part of their definition.
This has a placeholder,
<name>, for an embedded binding of a definition for an alias, and I didn't see any documentation of how<name>works, e.g. whether I should be definingCARGO_ALIAS_brorCARGO_ALIAS_BR.That is why I put up the PR.
Uh oh!
There was an error while loading. Please reload this page.
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.
After reviewing the broader docs, I do see other occurrences of the use of
<name>to describe a placeholder, but whether or not it is going to some kind of case-tranformation (e.g. mapping to lower-case, as is done forCARGO_ALIAS_<name>) is almost never indicated. (One rare exception is the discussion ofCARGO_REGISTRIES_<name>_TOKEN.)Many of the other occurrences of
<name>are places where there is a general template being described, e.g.[profile.<name>],[registries.<name>.index], etc, and<name>occurs multiple times in those contexts.CARGO_ALIAS_<name>is an exception to that pattern, which probably why I thought it deserved some discussion.Some of those cases are places where
<name>is used in a context where a lower-case form is expected, and then there are sometimes subcontexts where a environment variable likeCARGO_PROFILE_<name>_CODEGEN_UNITSorCARGO_PROFILE_<name>_DEBUGis then introduced.My current assumption is that the reader is expected to do a case-mapping in all such occurrences; e.g. mapping to lower-case for the occurrences in a toml file, and mapping to upper-case for occurrences when embedded in the name of an environment variable.
I'll open a general issue. Given that there are 53 occurrences of the string
<name>in the reference.html, I have sympathy for the idea that we probably prefer not to address this in an ad hoc manner. But on the other hand, it might be easiest to just blanket the appropriate places with the addendum "where is the name in all capital letters", analogous to what was done in the discussion ofCARGO_REGISTRIES_<name>_TOKEN.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.
Spawned off into #16532
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 would lean towards closing in favor of #16532, particularly if knowing how to convert
<name>was what led to this.