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

Call virtualenv by module #2198

Merged
merged 3 commits into from May 16, 2018

Conversation

Projects
2 participants
@uranusjr
Member

uranusjr commented May 15, 2018

I think this is actually a regression?

@uranusjr uranusjr force-pushed the call-virtualenv-by-module branch from 202182a to 4fee8ec May 15, 2018

@uranusjr uranusjr force-pushed the call-virtualenv-by-module branch from 4fee8ec to 55d77b2 May 15, 2018

@techalchemy

This might be a regression, but actually won't it just always

@@ -887,7 +887,7 @@ def do_create_virtualenv(python=None, site_packages=False):
# The user wants the virtualenv in the project.
if project.is_venv_in_project():
cmd = [
'virtualenv',
sys.executable, '-m', 'virtualenv',

This comment has been minimized.

@techalchemy

techalchemy May 15, 2018

Member

We probably want to use which('virtualenv') and fallback to sys.executable in case we can't find virtualenv or something, no?

This comment has been minimized.

@uranusjr

uranusjr May 16, 2018

Member

I wonder what the merit is. Is it so the user can set virtualenv to, like, a “better virtualenv”? Using our own dependency (even if not vendored) feels a much safer, predictable solution to me. Also in practice virtualenv hasn’t been updated in almost two years (last in November 2016), so the extra which check is more than likely just wasted.

This comment has been minimized.

@techalchemy

techalchemy May 16, 2018

Member

yeah i thought about it more and this does guarantee that we get the one that got installed with pipenv, so I like this

@techalchemy techalchemy merged commit 89921f7 into master May 16, 2018

0 of 4 checks passed

buildkite/pipenv Build #90 started
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@uranusjr uranusjr deleted the call-virtualenv-by-module branch May 17, 2018

@techalchemy techalchemy moved this from Done to Needs Changelog in 2018.06.x Release Jun 16, 2018

@techalchemy techalchemy moved this from Needs Changelog to Done in 2018.06.x Release Jun 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment