Skip to content

Fixes for ruff rule PLW1508 #6367

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

Merged
merged 1 commit into from
Mar 21, 2025
Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Mar 21, 2025

Thank you for contributing to Pipenv!

The issue

#6339 failed to merge cleanly so this pull request fixes to violations of ruff rule PLW1508.

What is the thing you want to fix? Is it associated with an issue on GitHub? Please mention it.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

Copy link
Member

@matteius matteius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some more refinements to pipenv_activate if I recall correctly in: #6360

@cclauss cclauss mentioned this pull request Mar 21, 2025
2 tasks
return []
if pip_trusted_hosts := os.environ.get("PIP_TRUSTED_HOSTS"):
return pip_trusted_hosts.split(" ")
return []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.split(" ") seems far more dangerous than .split() in the instance of multiple contiguous spaces or other whitespace like tab-separated data so an alternative might be:

return os.environ.get("PIP_TRUSTED_HOSTS", "").split()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats fine -- we want to make it the best it can be -- I tried here but has the same issue you are pointing out: https://github.com/pypa/pipenv/pull/6360/files#diff-827e674fd6d9a49bf96cdf22a01b6423dd7e92214db5836658108c4e35fa53d0L146-L150

I can rebase to what you determine is best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have merge rights so please merge or close as you see fit.

@cclauss cclauss force-pushed the fix-for-ruff-rule-PLW1508 branch from 9905520 to 1efaf48 Compare March 21, 2025 15:34
@matteius matteius merged commit 2989262 into pypa:main Mar 21, 2025
19 checks passed
@cclauss cclauss deleted the fix-for-ruff-rule-PLW1508 branch March 21, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants