-
Notifications
You must be signed in to change notification settings - Fork 966
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 new overload for Ordinalize that accepts a CultureInfo #595
Add new overload for Ordinalize that accepts a CultureInfo #595
Conversation
@aloisdg Do you have any idea if this is going to be picked up by anyone? |
@tiesmaster I dont know. I look again and it is great. I am bit out of time and knowledge to check for the build script. (I had the same problem #569 ) |
@hazzik Thnx for the fix! That's quite embarrassing 😊 It's still failing since the new overload is not in the |
@tiesmaster just add. |
per discussion on PR Humanizr#595.
Have you tried rebasing based on the latest dev? |
@hangy Thnx for the suggestion, I tried that, but didn't work. Finally, I was able to run the API approver locally, and found out that the order is important in the file. So that's fixed now, and the check is 💚 :) |
and associated tests.
@tiesmaster I believe a few overloads are missing from the public api.txt? |
added in previous commit.
@onovotny Ah, forgot the public api.txt again, sorry. Added that, and it passes locally now, so should pass CI. Had to switch from my Mac to Windows machine, as the public api is not verified on mac 😳. |
Thanks! |
per discussion on PR Humanizr#595.
This fixes #593, or at least, this is a start with that (@aloisdg asked me to put my preliminary work in a PR).
I've checked all boxes from the checklist, except for the last one (running the build script), since that's not working for me (I get nuget restore errors). I'll drop in the gitter chat to see if I can resolve those.