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

Add warning for potential extras_require misconfiguration #3481

Merged
merged 5 commits into from
Aug 6, 2022

Conversation

frenzymadness
Copy link
Contributor

Fixes: #3467

Summary of changes

See the linked issue, all details are there.

Closes #3467

Pull Request Checklist

@frenzymadness
Copy link
Contributor Author

Some of the jobs in CI are already green so this shouldn't break setuptools. I'm gonna add one more test.

Copy link
Contributor

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Thank you very much @frenzymadness for the work in this improvement. I added a few review comments/suggestions (but please feel free to disagree or propose alternatives).

@frenzymadness
Copy link
Contributor Author

I believe I addressed all the comments.

@frenzymadness
Copy link
Contributor Author

I have one fixup commit there. Don't forget to squash it to the previous one before merging. Thanks for helping me with this!

@abravalheri
Copy link
Contributor

abravalheri commented Aug 6, 2022

I was writing the following message before I noticed that I did a mistake in the refactoring and went back to correct it:

Hi @frenzymadness, thank you very much for the modifications!
I did some refactoring on the file to better integrate with your changes (I also refactored a little bit your code). I hope you don't mind. (The idea was to speed up the ping pong review process, please feel free to change it, if you think it is not appropriate).

But I guess you already noticed that 🤣

I have one fixup commit there. Don't forget to squash it to the previous one before merging. Thanks for helping me with this!

Ok, I will do it, thank you for the pointer.

@abravalheri
Copy link
Contributor

Thank you very much @frenzymadness, I plan to merge this PR together with a few other ones in a new release soon.

@abravalheri abravalheri merged commit ef6fd97 into pypa:main Aug 6, 2022
abravalheri added a commit that referenced this pull request Aug 6, 2022
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.

[FR] Improve error feedback for improper one-liner requirement with environment markers in setup.cfg
4 participants