Skip to content

Add warning for potential extras_require misconfiguration#3481

Merged
abravalheri merged 5 commits into
pypa:mainfrom
frenzymadness:extras_misconfig
Aug 6, 2022
Merged

Add warning for potential extras_require misconfiguration#3481
abravalheri merged 5 commits into
pypa:mainfrom
frenzymadness:extras_misconfig

Conversation

@frenzymadness

Copy link
Copy Markdown
Contributor

Fixes: #3467

Summary of changes

See the linked issue, all details are there.

Closes #3467

Pull Request Checklist

Comment thread setuptools/tests/config/test_setupcfg.py Outdated
@frenzymadness

Copy link
Copy Markdown
Contributor Author

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

Comment thread setuptools/tests/config/test_setupcfg.py Outdated

@abravalheri abravalheri left a comment

Copy link
Copy Markdown
Contributor

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).

Comment thread setuptools/config/setupcfg.py Outdated
Comment thread setuptools/tests/config/test_setupcfg.py Outdated
Comment thread setuptools/tests/config/test_setupcfg.py Outdated
Comment thread setuptools/config/setupcfg.py Outdated
@frenzymadness

Copy link
Copy Markdown
Contributor Author

I believe I addressed all the comments.

@frenzymadness

Copy link
Copy Markdown
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

abravalheri commented Aug 6, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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