Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
[doc] Document VIRTUAL_ENV environment variable (GH-21970)#21970
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
andresdelfino commented Aug 26, 2020 • edited by miss-islington
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by miss-islington
Uh oh!
There was an error while loading. Please reload this page.
miss-islington commented Jan 28, 2021
Thanks @andresdelfino for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
miss-islington commented Jan 28, 2021
Thanks @andresdelfino for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8. |
bedevere-bot commented Jan 28, 2021
GH-24362 is a backport of this pull request to the 3.9 branch. |
bedevere-bot commented Jan 28, 2021
GH-24363 is a backport of this pull request to the 3.8 branch. |
(cherry picked from commit 3584d4b) Co-authored-by: Andre Delfino <adelfino@gmail.com>
(cherry picked from commit 3584d4b) Co-authored-by: Andre Delfino <adelfino@gmail.com>
(cherry picked from commit 3584d4b) Co-authored-by: Andre Delfino <adelfino@gmail.com>
pelson commented Oct 6, 2022
@vsajip - sorry to drag this up, but I believe this change is incorrect. It would be dangerous to use an environment variable to determine if a virtual environment is being used or not. I would never do this in production, and instead would check This recommendation is indeed already explicitly stated in the same file:
A simple counter example to the documentation would be: I therefore recommend that this commit be reverted. I would be happy to file a bug if that is required to revert. |
vsajip commented Oct 6, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Rather than doing that, I would propose to change the paragraph as follows:
|
pelson commented Oct 6, 2022
I take your lead on this, but to me that sounds like quite a lot of words for not a lot of new detail. I definitely think the page in question could be reshuffled a bit to make this concise and easier to find the information quickly. Would you like me to submit a proposal? |
vsajip commented Oct 6, 2022
By all means. |
andresdelfino commented Oct 6, 2022
I think we shouldn't remove the mention to VIRTUAL_ENV (reverting this commit would have that effect), as it is helpful. Of course, I'm all for improving the correctness of the docs. |
vsajip commented Oct 6, 2022
Well, I made my suggestion in the way that I did because it
and it may be wordier than it needs to be, but I was thinking of spelling things out for any inexperienced users / users new to virtual environments. |
Automerge-Triggered-By: @vsajip