-
-
Notifications
You must be signed in to change notification settings - Fork 150
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
Allow not passing "--upgrade" to session.install
#172
Conversation
This is a great point. I'm not sure why I ended up going with @dhermes WDYT? |
I think it should be off by default and have an |
nox/sessions.py
Outdated
@@ -242,8 +242,11 @@ def install(self, *args, **kwargs): | |||
|
|||
if "silent" not in kwargs: | |||
kwargs["silent"] = True | |||
upgrade = kwargs.pop("upgrade", True) |
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 am all for this but I'd make False
the default
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.
That's a backwards incompatible change, but I'm cool with if you're cool with it too.
At that point we could just pass it through like every other pip parameter, e.g.: session.install("--upgrade", "-r", "requirements.txt") |
Ha, DERP. Good call |
Cool, sound good to you, @s0undt3ch? |
Sorry, just woke and now I read the whole list of comments. |
I'll update the PR. |
PR updated. |
Seems like some documentation needs to be updated as well: https://github.com/theacodes/nox/search?q=upgrade&unscoped_q=upgrade |
Docs updated. |
I don't really know the reasoning why `--upgrade` was added by default to the `pip` call in `session.install`. I reason against it, and such flag should be passed explictily
Thank you, @s0undt3ch! |
I don't really know the reasoning why
--upgrade
was added by default to thepip
call insession.install
.I would reason against it, but, since it has been the default, at least allow the user not to upgrade.