Use I18n and add Japanese locale.#259
Conversation
|
Hi @tomoyuki28jp, I'm wondering what the state of this work is? Do you plan to complete and merge it? |
|
@stravabr I think I've done with it. When I run rspec tests on my local machine, it seems some tests fail before I make any changes. (Source code is forked from the master branch on github.) So I've modified tests which are directly related to my changes. (But I might be wrong!) |
|
@stravabr If I better do more work, please let me know. But I'm not sure if the owner will merge this pull request or not because they haven't responded to this yet. (It's been a month so far.) |
|
Sorry I haven't had a chance to review & merge new features. I'll try and set aside some time for it soon. This looks promising, and it looks like it would apply cleanly. I think the i18n gem would need to be a real dependency, not just a "development dependency". Was there a reason the gemspec has it that way? |
|
@avit Thank you for taking a look at my pull request.
Ah, sorry. I just don't have much experience on developing a gem package. Is this a right way to fix it? tomoyuki28jp@9be09f7 |
|
Right now we have zero runtime gem dependencies, but I think this is a very common gem and it would be worth adding for this functionality. You can change it to |
|
@avit Thanks for your advice! I made the change. tomoyuki28jp@e09e2a4 |
|
👍 thanks for taking a work on this ! |
|
Hey @tomoyuki28jp, I got the cause for the errors in ja tests: I18n default time format is memoized here: https://github.com/seejohnrun/ice_cube/blob/master/lib/ice_cube.rb#L71, so when you run the whole suite it takes the value the first time (english format) and keeps on with that even if you change the I'm not particularly happy to see such kind of class-level memoizing, I suspect that will cause thread-safety issues. I think that we should remove that, what do you think @avit? |
|
@tomoyuki28jp @avit Any more progress/plans on sorting out these tests and getting this merged? Is there any way I can help? |
|
@stravabr Unfortunately, I have no idea :( |
|
I just sent a PR to @tomoyuki28jp to tomoyuki28jp#1 that fixes the tests as per my comment above. There are some failing tests yet, but I cannot find the cause. If any of you can put some time on this @stravabr @avit, we might get this task finished for once! Regards, |
|
What is the status of this? Will it be possible to use I18n? |
|
Cool @dgilperez. Maybe I am stupid but how do I add a PR in the Gemfile? |
|
You can refer to a particular branch or commit in your Gemfile like this: Also, peeking at the network graph from my fork, I see the nice guys at gocardless have continued work on that PR, adding some locales and a few other goodies. It may a better replacement, what do you think @isaacseymour? Is your fork production ready? |
|
Awesome. I'll take a look at that too then! |
|
Yep, we've been using the gocardless fork in production, it's not hugely On Thu, 1 Oct 2015 20:18 Emil Karlsson notifications@github.com wrote:
|
|
Thanks @isaacseymour !!! Giving it a try ... |
|
@tomoyuki28jp Sorry, I have been away from this project for some time. I18n was added as an optional dependency in the meantime, and I would be happy to merge your contribution for Japanese translation. If you still find it useful, please see the latest master branch and add your jp.yml on it. We can reopen this PR or make a new one. |
6 participants
I made changes based on the work made by @joelmeyerhamme ( https://github.com/joelmeyerhamme/ice_cube/tree/international ).
#68