New branch, removes empty flag in SVN install command. Fixes SVN inst… #5993
Conversation
|
@rjsilver If you can check the ruff lint and add a news fragment, I may be able to include this in tonight's release. |
|
Actually I am pushing a fix to the ruff lint to main -- has nothing to do with this PR, but a news fragment would be good. |
|
Hey Matt,
Thanks for looking at this so quickly, is there documentation for how to do the news fragment? I'm unfamiliar with this method.
Thanks,
Ryne
…________________________________
From: Matt Davis ***@***.***>
Sent: Tuesday, October 24, 2023 3:25 PM
To: pypa/pipenv ***@***.***>
Cc: Ryne Joseph Silver ***@***.***>; Author ***@***.***>
Subject: Re: [pypa/pipenv] New branch, removes empty flag in SVN install command. Fixes SVN inst… (PR #5993)
@rjsilver<https://github.com/rjsilver> If you can check the ruff lint and add a news fragment, I may be able to include this in tonight's release.
—
Reply to this email directly, view it on GitHub<#5993 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHZTOVZ5QQMIXFYLGIDP2WTYBAW5DAVCNFSM6AAAAAA6OJHK4KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZYGA3DIOJQGU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@rsilver23 Basically create a file in the news directory |
|
@rjsilver oh hey -- there is a problem I just noticed, which is you are modifying the pip internals. We don't want to do that, if there was truly a reason to do so, we would provide a patch, but I think in this case you mentioned it is something to do with click? |
|
@rjsilver I wonder if pypa/pip#11050 is related? |
|
@matteius pypa/pip#11050 definitely looks related |
|
@matteius looks like this code already exists in a patch. Do you have a better recommendation where to put this? |
|
@rsilver23 So we vendor in pip to pipenv and we have some patches that we apply after the fact. This feels like a case where pip itself would accept this patch, and the original PR that fixes it was tagged with needs a rebase. Perhaps you could open a new PR against pip -- the problem is, if I merge this as-is, the next time we vendor in a new version of pip it would get overwritten. |
| @@ -288,17 +288,22 @@ def fetch_new( | |||
| display_path(dest), | |||
| ) | |||
| if verbosity <= 0: | |||
| flag = "--quiet" | |||
| cmd_args = make_command( | |||
There was a problem hiding this comment.
This definitely looks like something that would not encounter much resistance in upstream.
|
Hi! FYI the original PR opened into pip repo and mentioned here has been merged. |
|
You mean this one? pypa/pip#11051? |
|
That is great to hear -- we already regular vendor in pip, so I will close this PR as the plan is to go with the update in pip. |
…alls
Thank you for contributing to Pipenv!
The issue
This branch fixes pipenv install of SVN repos. Pipenv was adding and extra empty argument when the verbosity flag was <= 0. This empty string was causing an error with SVN checkout commands.
The fix
This pull request removes the empty flag from the command arguments when the verbosity is not set.