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

fix: more focused output if setup.py is empty #2629

Closed
wants to merge 1 commit into from

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Apr 1, 2021

Summary of changes

Followup to #2608. Makes sure an unrelated ValueError is not caught by the try/except, then removes the extra exception printout. Also moves the msg to the line above to avoid a double message printout in the traceback.

Pull Request Checklist

Followup to pypa#2608. Makes sure an unrelated ValueError is not caught by the try/except, then removes the extra exception printout. Also moves the msg to the line above to avoid a double message printout in the traceback.
@henryiii
Copy link
Contributor Author

henryiii commented Apr 1, 2021

This should already be tested, can add more if needed, also don't know if it's large enough to warrant a changelog entry, happy to add one if it is.

@henryiii
Copy link
Contributor Author

henryiii commented Apr 1, 2021

ERROR: HTTP error 404 while getting https://github.com/pypa/pip/archive/master.zip

Don't think I caused this... :) I'll wait to see what the feedback / interest is before retriggring.

@jaraco jaraco closed this Apr 3, 2021
@jaraco jaraco reopened this Apr 3, 2021
Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

The more I think about it, I think I prefer the existing implementation to the change proposed here. Let's just stick with what we have. If it turns out to cause actual consternation, we can revisit, but for now, I think the existing implementation is satisfactory.

Still, thanks for the contribution and consideration.

@@ -97,16 +97,16 @@ def _get_immediate_subdirectories(a_dir):


def _file_with_extension(directory, extension):
matching = (
matching = [
Copy link
Member

Choose a reason for hiding this comment

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

I saw the conversation that inspired this change, but if I hadn't, I wouldn't understand why this was here and I'd be reluctant to accept the change. Moreover, without that context or seeing this diff, I'd be inclined to change this back to a generator expression to build on lazy evaluation.

I'm slightly inclined to think this code is becoming overly-protective and it may not be worth the effort to try to protect against unexpected value errors or to hide them from the output.

@jaraco jaraco closed this Apr 3, 2021
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