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

Combine components under Localisation namespace by language rather than by component type #206

Closed
hazzik opened this issue Apr 13, 2014 · 8 comments

Comments

@hazzik
Copy link
Member

hazzik commented Apr 13, 2014

/Humanizer/Localisation/

/Humanizer/Localisation/ar/ArabicFormatter
/Humanizer/Localisation/ar/ArabicNumberToWordsConverter

/Humanizer/Localisation/ru/RussianFormatter
/Humanizer/Localisation/ru/RussianNumberToWordsConverter
/Humanizer/Localisation/ru/RussianGrammaticalNumber
/Humanizer/Localisation/ru/RussianGrammaticalNumberDetector

etc

hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 13, 2014
hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 13, 2014
hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 13, 2014
hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 17, 2014
hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 27, 2014
hazzik added a commit to hazzik/Humanizer that referenced this issue Apr 27, 2014
@hazzik hazzik added the jump in label Aug 28, 2014
@hazzik hazzik added this to the V2 milestone Aug 28, 2014
@hazzik
Copy link
Member Author

hazzik commented Aug 28, 2014

Related to #59

@akamud
Copy link
Contributor

akamud commented Oct 8, 2014

@hazzik, I looked into your issue-206 branch, it looks like you already finished this. Is there something else that needs to be done or are we just waiting for V2?

@hazzik
Copy link
Member Author

hazzik commented Oct 8, 2014

just waiting

@MehdiK
Copy link
Member

MehdiK commented Oct 10, 2014

There were a few confusions are how we wanted to do it. IIRC we couldn't find a win-win situation and there was always a compromise. I am still not 100% convinced which way is better tbh.

The PR will also have to be updated with the later localisation additions.

@clairernovotny
Copy link
Member

Is this still in scope for v2? I'd like to get a beta out by the end of the month as the current dev branch adds CoreCLR support.

@MehdiK
Copy link
Member

MehdiK commented Oct 26, 2015

When we were discussing this solution layout, I have very vague recollection but from the memory some things didn't quite fit into the proposed solution. Not that they do in the current structure but I guess my point is that this solved one problem and introduced another one. So I'm torn on it. I'd vote to keep it as is if we don't have a strong argument for this structure. Thoughts?

@clairernovotny
Copy link
Member

I don't have any strong thoughts on this. There's something to be said for both sides. As far as I can see, aside from the interfaces/abstract base classes, the actual localization implementations are all internal, so moving it around should not be a breaking change.

@clairernovotny
Copy link
Member

I'm closing this out, leaving things as-is for now. We can revisit if there's reason to later.

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

No branches or pull requests

4 participants