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

Remove localize formatting rules #650

Closed
wants to merge 2 commits into from
Closed

Remove localize formatting rules #650

wants to merge 2 commits into from

Conversation

yukihariguchi
Copy link

Since Formatting#localize_formatting_rules prevent from overwriting rules, this patch is going to propose to remove Formatting#localize_formatting_rules.

First of all, this method was introduced at #101. But it doesn't make sense to me and unexpected behavior. Not only '円' but also '圓'(it's more formal) or even 'yen' can be used.

@coveralls
Copy link

coveralls commented Aug 18, 2016

Coverage Status

Coverage decreased (-0.003%) to 99.376% when pulling c20176c on yukihariguchi:remove_localize_formatting_rules into 5630a70 on RubyMoney:master.

@antstorm
Copy link
Contributor

@yukihariguchi thanks for the PR, looks legit from what I can tell. Do you have any examples of this? Wiki or any other resource?

Also, CHANGELOG and AUTHORS is updated when releasing a new version, so can you please drop that last commit?

@antstorm
Copy link
Contributor

@yukihariguchi any chance you can rebase this PR and answer my question? Thanks!

@antstorm
Copy link
Contributor

@pwim do you have any suggestions here?

@pwim
Copy link
Contributor

pwim commented Sep 23, 2018

I've taken a look at some of the top ecommerce sites in Japan.

Site Symbol
http://www.amazon.co.jp/ ¥
https://www.yodobashi.com/ ¥
https://www.rakuten.co.jp/
http://zozo.jp/ ¥
https://www.bellemaison.jp/ ¥
http://kakaku.com/ ¥
https://auctions.yahoo.co.jp/
https://www.mercari.com/jp/ ¥
https://www.uniqlo.com/jp/ ¥

So as of 2018, it seems that using ¥ rather than 円 is more common. Maybe the situation was different back in 2011 when I first made #101, or maybe I was misguided and it wasn't necessary.

Given this, I'm fine for my application displaying "¥500" instead of "500円", and so I'm not opposed to this PR.

This is a breaking change, so it should probably go into the next major version.

@pwim
Copy link
Contributor

pwim commented Sep 23, 2018

Additionally, I'd call out that this changes how Japanese yen is formatted explicitly in the changelog. Something like

Format Japanese Yen the same across all locales. Previously, when I18n.locale == :ja, JPY would be displayed using 円 (e.g. "500円"), whereas in other locales it was displayed as using ¥ (e.g. "¥500"). Now it is always displayed using ¥, even when the locale is set to ja.

@antstorm
Copy link
Contributor

@pwim since I'm not familiar with the matter I trust your judgment. Thank you for pointing out that it's a breaking change and sketching a text for changelog, really appreciate that!

@yukihariguchi can you please rebase the PR?

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.

5 participants