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

Changed OrdinalizeExtensions to better accommodate localisations. #217

Merged
merged 4 commits into from
Apr 15, 2014

Conversation

akamud
Copy link
Contributor

@akamud akamud commented Apr 15, 2014

Added pt-br Ordinalize localisation.

@akamud
Copy link
Contributor Author

akamud commented Apr 15, 2014

Since @thunsaker said Spanish rules are exactly the same, I added Spanish localisation too. Hope I didn't get something wrong.

Would be great if someone could validate all these changes. I'll check tomorrow if there are any problems.

@thunsaker
Copy link
Contributor

Spanish additions look good, (one inline comment). Thanks for doing it.

[InlineData("102", "102º")]
[InlineData("103", "103º")]
[InlineData("1001", "1001º")]
public void OrdinalizeString(string number, string ordinalized)
Copy link
Member

Choose a reason for hiding this comment

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

Rename this to Ordinalize please

Never mind. I see the reason now.

@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

Thanks @akamud. Great work.

Please leave us a comment when the feedback is applied.

…l Gender for Ordinalizer.

Fixed some typos and code clean-up.
@akamud
Copy link
Contributor Author

akamud commented Apr 15, 2014

Please check if everything is correct now.

I'll be back home tonight if there's still something wrong.

@MehdiK MehdiK merged commit b353f33 into Humanizr:master Apr 15, 2014
@MehdiK
Copy link
Member

MehdiK commented Apr 15, 2014

Never mind. I made the changes and pushed. Please review them e5c631c

Thanks for the great work @akamud.

@akamud
Copy link
Contributor Author

akamud commented Apr 15, 2014

Thank you for making them @MehdiK, I wouldn't be able to fix them until later tonight.

@MehdiK
Copy link
Member

MehdiK commented Apr 19, 2014

Thanks for the contribution. This is now available on NuGet as v1.23.1.

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

Successfully merging this pull request may close these issues.

4 participants