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

adds missing resources/tests for the Romanian locale #356

Merged
merged 1 commit into from
Dec 11, 2014

Conversation

cristi-badila
Copy link
Contributor

Also modifies the RomanianFormatter so as to make it clearer what's happening and remove resource duplication

@cristi-badila
Copy link
Contributor Author

In regards to the modification of the RomanianFormatter, I suppose I should add the motivation behind why I decided to change it: I just wanted Humanizer to work for the Romanian culture but some resources were missing which had the consequence of calls to Humanize blowing up. I quickly decided to add the missing resources but as soon as I did this I became bored of having to add the same resource twice (once with and once without the preposition) so I figured there must be a better way to do this (which I think I implemented). Also I think the existing GetResourceKey implementation (in the RomanianFormatter) is quite opaque and one doesn't really understand why it's doing what it does. Anyway, I'm open to a discussion on this.

… RomanianFormatter so as to make it clearer what's happening and remove resource duplication
@MehdiK
Copy link
Member

MehdiK commented Dec 11, 2014

This is clean and nice. No need for discussion. Thanks for the addition.

MehdiK added a commit that referenced this pull request Dec 11, 2014
adds missing resources/tests for the Romanian locale
@MehdiK MehdiK merged commit e2d9804 into Humanizr:master Dec 11, 2014
@MehdiK
Copy link
Member

MehdiK commented Dec 18, 2014

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

@cristi-badila
Copy link
Contributor Author

My pleasure. I'm always glad when I can give back to the Open Source community. Thanks for the update.

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.

2 participants