Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support version specifiers in GH action #3265
Support version specifiers in GH action #3265
Changes from 3 commits
feada3f
e7417d3
63fec34
e537d41
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah damn, I didn't notice this comment before I made mine 😄 I can change it, though I would like to note that isdecimal is less wide check than isdigit so it might be better choice if we're switching it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do
VERSION[0].isdecimal()
here though that's rather wide due to it supporting all Unicode characters in the Unicode General Category “Nd”. Personally, I tend to avoid isdecimal/isdigit/isnumeric methods because they evaluate to True for more than I actually want but I can change this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's fine either way, since we only care about recognising version specifier characters, which won't be decimal or digits. But no biggie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do packaging tools normalize non-ascii digits? If so, I'd prefer
isdecimal()
otherwise I'd prefer being stricter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip
doesn't handle Unicode numbers:So in the end, it shouldn't matter whether we use
isdecimal()
or thein
check as it will fail anyway.