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

Adhere to PEP 685 when evaluating markers with extras #545

Merged
merged 1 commit into from
May 12, 2022

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented May 10, 2022

Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

LGTM, just want to simplify the code a bit.

packaging/markers.py Outdated Show resolved Hide resolved
@brettcannon
Copy link
Member

FYI I have an in-progress PR that also normalizes the value so that the string representation is fully normalized. But I like the solution for this part of it by @hroncok more than mine. 🙂

@hroncok
Copy link
Contributor Author

hroncok commented May 10, 2022

There is a fixup commit now. Please squash when (or before) merging or tell me to do it when ready.

@pradyunsg
Copy link
Member

Please go ahead and squash.

I learnt this recently: If you make a force-push with a single commit amended, it's possible to see what you've changed in the force-push -- by clicking "force-pushed" text in the event that GitHub adds. This is useful for exactly the situation we have here -- a single commit fixup. :)

@brettcannon brettcannon self-requested a review May 11, 2022 22:44
@brettcannon
Copy link
Member

The change LGTM, so like Pradyun said, please go ahead and squash. 🙂

@hroncok
Copy link
Contributor Author

hroncok commented May 12, 2022

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants