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 colons from mail template since they are within the translations ... #2278

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@JDutil
Member

JDutil commented Dec 5, 2012

...causing :: to display.

@joneslee85

This comment has been minimized.

Show comment
Hide comment
@joneslee85

joneslee85 Dec 7, 2012

@JDutil IMHO, we should leave it there, and fixing the translation file instead

joneslee85 commented on 076dc6b Dec 7, 2012

@JDutil IMHO, we should leave it there, and fixing the translation file instead

This comment has been minimized.

Show comment
Hide comment
@JDutil

JDutil Dec 7, 2012

Owner

@joneslee85 I had considered that myself as well. I was going to ask peoples opinions on this as well, which I'm open to. My decision to go with updating the template was based on my thought that it could be possible that some languages may make more sense to have some sort of different separator symbol or order to the translation. Therefore I thought it would be more flexible for i18n to leave the : in the translation rather than the template.

Owner

JDutil replied Dec 7, 2012

@joneslee85 I had considered that myself as well. I was going to ask peoples opinions on this as well, which I'm open to. My decision to go with updating the template was based on my thought that it could be possible that some languages may make more sense to have some sort of different separator symbol or order to the translation. Therefore I thought it would be more flexible for i18n to leave the : in the translation rather than the template.

This comment has been minimized.

Show comment
Hide comment
@JDutil

JDutil Dec 7, 2012

Owner

Although if the order matters in the language this may actually make more sense for the total to be a parameter of the translation key. Thoughts?

Owner

JDutil replied Dec 7, 2012

Although if the order matters in the language this may actually make more sense for the total to be a parameter of the translation key. Thoughts?

@GeekOnCoffee

This comment has been minimized.

Show comment
Hide comment
@GeekOnCoffee

GeekOnCoffee Dec 10, 2012

Contributor

I like the idea of having the total as a parameter of the translation key...

Contributor

GeekOnCoffee commented Dec 10, 2012

I like the idea of having the total as a parameter of the translation key...

@JDutil

This comment has been minimized.

Show comment
Hide comment
@JDutil

JDutil Dec 10, 2012

Member

I've just pushed an update that passes the totals as a parameter for more flexibility in the translations. If people prefer what I had before w/just removing the extra colon I can just roll the the commit back.

Member

JDutil commented Dec 10, 2012

I've just pushed an update that passes the totals as a parameter for more flexibility in the translations. If people prefer what I had before w/just removing the extra colon I can just roll the the commit back.

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Dec 10, 2012

Member

I like it how it is now. Will merge.

Member

radar commented Dec 10, 2012

I like it how it is now. Will merge.

@radar radar closed this in 0ead239 Dec 10, 2012

radar added a commit that referenced this pull request Dec 10, 2012

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