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

tweaking so that the module gives us a way to make multiple i18n objects #323

Closed
wants to merge 3 commits into from
Closed

tweaking so that the module gives us a way to make multiple i18n objects #323

wants to merge 3 commits into from

Conversation

julka
Copy link

@julka julka commented Jul 18, 2017

It's a common process to have different i18n files for different parts of a project. Sometimes we want to be able to use different i18n configurations in environments and jump back and forth between which one we're using (think async build process). This change would be non-breaking for those who are using index.js (implicitly or explicitly) but also allow to build multiple unique i18n objects by going to i18n.js directly when needed.

Is this something you'd consider supporting in this project?

…cts when needed -- this change should be backward compatible for anyone who isn't including the i18n.js file explitictly, instead of defaulting to the index.js file
@coveralls
Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage decreased (-96.9%) to 1.046% when pulling a230767 on julka:julka/object_builder into 11d89da on mashpie:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-96.9%) to 1.046% when pulling a230767 on julka:julka/object_builder into 11d89da on mashpie:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-96.9%) to 1.046% when pulling a230767 on julka:julka/object_builder into 11d89da on mashpie:master.

@mashpie
Copy link
Owner

mashpie commented Jul 19, 2017

Definetly - is on my roadmap too. But tests should pass. Would you mind checking?

@coveralls
Copy link

coveralls commented Jul 19, 2017

Coverage Status

Coverage increased (+0.004%) to 97.912% when pulling a28ef4b on julka:julka/object_builder into 11d89da on mashpie:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.912% when pulling a28ef4b on julka:julka/object_builder into 11d89da on mashpie:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.912% when pulling a28ef4b on julka:julka/object_builder into 11d89da on mashpie:master.

@coveralls
Copy link

coveralls commented Jul 19, 2017

Coverage Status

Coverage increased (+0.004%) to 97.912% when pulling a28ef4b on julka:julka/object_builder into 11d89da on mashpie:master.

@@ -1,4 +1,4 @@
var i18n = require('../i18n'),
var i18n = require('../index'),
Copy link
Author

@julka julka Jul 19, 2017

Choose a reason for hiding this comment

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

Alternately, I could change the require('../i18n') bits to require('../i18n')() instead of require('../index') in most spots, but I think that could take more memory to test since each spot that does this would be creating it's own i18n object.

@julka
Copy link
Author

julka commented Jul 19, 2017

Glad to @mashpie! How that?

@julka
Copy link
Author

julka commented Jul 31, 2017

Hey @mashpie, is there anything more I can do for this PR? Thanks!

@mashpie
Copy link
Owner

mashpie commented Jul 31, 2017

nice & thanks! Looks to be ready for merge and review... too busy atm - hope for weekend :)

@julka
Copy link
Author

julka commented Aug 14, 2017

Hey @mashpie. Just checking in. Is there anything more I can do? Thanks!

@mashpie
Copy link
Owner

mashpie commented Aug 14, 2017

No thanks, all fine. I'll be traveling this week but will try to release latest next monday. There are a couple of pr I wanted to merge in that release also.

@ChrisCinelli
Copy link

Any news?

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