-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
Session.posargs should be immutable #400
Comments
Thanks for creating the issue @rmorshea ! Here's an example to illustrate the problem: import nox
@nox.session
def test(session):
session.posargs.extend(["-x"])
session.install("pytest")
session.run("pytest", *session.posargs)
@nox.session
def lint(session):
args = [*session.posargs, "src"]
session.install("flake8")
session.run("flake8", *args) The lint session will fail with |
In the context of #397, we discussed two approaches to solve this:
On further thought, I'm worried that option 2 would break users that expect @nox.session
def test(session):
args = session.posargs + ["--full-trace", "--verbose"]
# TypeError: can only concatenate tuple (not "list") to tuple It's true that typically Option 1 will require some updates to the test suite, but should not break existing Noxfiles. See #397 (comment) for a patch that updates the tests accordingly. |
@cjolowicz good point about the list additions. I think I do that myself actually. Copying the list might be the right approach here. I feel like there's a lot more people who will be harmed by changing this to a tuple than would be protected by its immutability. |
Yes, flipping through a few examples most of them use generic sequence methods like And I think I might have found a superspreader for one common pattern... pip: @nox.session
def lint(session):
# type: (nox.Session) -> None
session.install("pre-commit")
if session.posargs:
args = session.posargs + ["--all-files"]
else:
args = ["--all-files", "--show-diff-on-failure"]
session.run("pre-commit", "run", *args) So while a tuple might have been best from the outset, given what's in the wild, copying the list seems like a reasonable approach. |
There's no reason that
Session.posargs
should be mutated by users.Originally posted by @cjolowicz in #397 (comment)
The text was updated successfully, but these errors were encountered: