-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
Fixed #35443 -- Changed ordinal to return negative integers as is. #18172
Conversation
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.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
You beat me to it. I like the test with |
Thanks :) I committed a fixup (8130b21) for the docs. Unsure what the policy for this is, but I assume everything is squashed when accepted? Didn't want to alter the history with a force push. |
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.
Thank you for your contribution! I’ve reviewed the code, tests, and documentation, and they all appear to follow the guidelines. However, for the pull request itself, the commits need to be squashed into a single commit. Once that’s done, it can proceed with the review.
Great! The commits are now squashed. Thanks for the help. |
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.
Thank you!
I think I would update to only support "positive" integers as I think it's clearer than "non-negative" and "0th" also doesn't make sense in natural language 👍
Previously, `-1` was converted to `"-1th"`. This has been updated to return negative numbers "as is", so that for example `-1` is converted to `"-1"`. This is now explicit in the docs. Co-authored-by: Martin Jonson <artin.onson@gmail.com>
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.
Thank you @S-Tornqvist ⭐
Trac ticket number
ticket-35443
Branch description
"-1st", "-2nd" and so on are incorrect words. Function
ordinal
in modulehumanize
has been changed to return negative number "as is", so that for example-1
is converted to"-1"
.The changes corresponds to alternative 1 of the suggested solutions in ticket #35443, which was accepted by Sarah Boyce.
Checklist
main
branch.