Skip to content
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

Added command line flag "--unsafe" to allow the pinning of #327

Closed
wants to merge 2 commits into from

Conversation

cyrusd
Copy link

@cyrusd cyrusd commented Mar 1, 2016

"unsafe" dependencies; unfortunately there are some valid
instances of needing to enforce this, it seems like setuptools
in particular goes through various phases of backwards
compatibility.

I wasn't 100% certain how to run the tests ("python setup.py test"
didn't seem to do the trick) but I did add a best guess at how
to test the new option.

"unsafe" dependencies; unfortunately there are some valid
instances of needing to enforce this, it seems like setuptools
in particular goes through various phases of backwards
compatibility.

I wasn't 100% certain how to run the tests ("python setup.py test"
didn't seem to do the trick) but I did add a best guess at how
to test the new option.

for ireq in unsafe_packages:
line = self._format_requirement(ireq, reverse_dependencies, primary_packages)
yield line
Copy link
Member

Choose a reason for hiding this comment

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

I like the idea for this flag. Rather than putting in an explicit comment here and repeat some of the code, I'd be fine simply setting the unsafe_packages set to the empty set (on line 62) if this flag is active. This will not make setuptools stand out from the rest (but that's the intention if this flag is used anyway).

@nvie
Copy link
Member

nvie commented Mar 2, 2016

To run the tests, run:

$ tox

@cyrusd
Copy link
Author

cyrusd commented Mar 3, 2016

Ok, updated the code to reflect your suggestions, I wasn't able to get the test for the 'unsafe' option to pass (indexing issues I think) but kind of ran out of time to look into it...

@alekzvik
Copy link
Contributor

Implemented in #377

@nvie nvie closed this Jul 12, 2016
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.

3 participants