-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Check environment variable PIPENV_CUSTOM_VENV_NAME #5203
Conversation
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.
Instead of asking os.getenv
directly, we should go through:
self.s.PIPENV_CUSTOM_VENV_NAME
All settings are found in environments.Settings
class.
Using this class conventions is also to document the variable, as other variables are documented there.
Setting Suppose I did: Now there is:
If I run pipenv in a new shell it's not going to have PIPENV_CUSTOM_VENV_NAME, hence it will look for an environment @matteius do you have an opinion here? |
@oz123 I am not sure I agree with that approach when its much easier to have a |
I am considering putting self.s.PIPENV_CUSTOM_VENV_NAME into activate in environment.py, that seems like the creating a venv context level that this is appropriate to. Waiting for more consensus around the pipfile bit. |
@otherjake The reading of the environment variable should happen in |
@matteius true. Putting it in |
in the same way as other environment variables. Update documentation to include the new variable.
…nto 4974-custom-venv-name
This obsoletes, #4974. This is a reminder for us to make sure we close that one too. |
in advanced docs.
…nto 4974-custom-venv-name
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.
Nice @otherjake !
This is so that PIPENV_CUSTOM_VENV_NAME does not leak to other tests.
@otherjake Thank you for your contribution! I added a small change to your test. Didn't want to bother you again with changes. Besides that, as usual 💯 points and a big thanks! |
Resolves #1226
Allows user to set a custom venv name
The fix
Check for PIPENV_CUSTOM_VENV_NAME environment variable, accept it as venv name at creation if it is set