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

Add new overload for Ordinalize that accepts a CultureInfo #595

Merged
merged 5 commits into from
Apr 4, 2019
Merged

Add new overload for Ordinalize that accepts a CultureInfo #595

merged 5 commits into from
Apr 4, 2019

Conversation

tiesmaster
Copy link
Contributor

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.

@tiesmaster
Copy link
Contributor Author

@aloisdg Do you have any idea if this is going to be picked up by anyone?

@aloisdg
Copy link
Contributor

aloisdg commented Nov 23, 2016

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

@tiesmaster
Copy link
Contributor Author

@hazzik Thnx for the fix! That's quite embarrassing 😊 It's still failing since the new overload is not in the ...public_api.approved.txt. Shall I just add it to it, or is there a different process for that?

@hazzik
Copy link
Member

hazzik commented Dec 8, 2016

@tiesmaster just add.

hangy pushed a commit to hangy/Humanizer that referenced this pull request Dec 9, 2016
@hangy
Copy link
Contributor

hangy commented Dec 12, 2016

Have you tried rebasing based on the latest dev?

@tiesmaster
Copy link
Contributor Author

@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 💚 :)

Thijs Brobbel and others added 3 commits April 3, 2019 20:43
@tiesmaster tiesmaster changed the base branch from dev to master April 3, 2019 18:43
@tiesmaster tiesmaster reopened this Apr 3, 2019
and associated tests.
@clairernovotny
Copy link
Member

@tiesmaster I believe a few overloads are missing from the public api.txt?

@tiesmaster
Copy link
Contributor Author

@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 😳.

@clairernovotny clairernovotny merged commit 746263d into Humanizr:master Apr 4, 2019
@clairernovotny
Copy link
Member

Thanks!

jvanrhyn pushed a commit to jvanrhyn/Humanizer that referenced this pull request Sep 10, 2020
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.

Add overload for specifying the culture for Ordinalize() extension method
5 participants