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

RFC: i18n for accounts-base #82

Closed
softwarerero opened this issue Feb 11, 2014 · 10 comments
Closed

RFC: i18n for accounts-base #82

softwarerero opened this issue Feb 11, 2014 · 10 comments

Comments

@softwarerero
Copy link
Contributor

The accounts-base package unfortunately does not throw error codes but english texts in case of an error. This is not directly accounts-entry's problem but we could work around it. There are a few calls to Session.set('entryError', error.reason) which could be replaced by Session.set('entryError', (i18n err.reason.replace ".", "") || err.reason) and a translation could be provided like "Email already exists": "Email ya existe".

Do you think it makes sence to have this in accounts-entry? Should a provide a pull request for this?

@queso
Copy link
Contributor

queso commented Feb 11, 2014

I am not opposed to it, but would it be cleaner to have the errors thrown properly from base? I don't think the MDG would oppose such a request for internationalization reasons.

@queso
Copy link
Contributor

queso commented Feb 11, 2014

I am also inclined to accept #78 before we solve this issue... The functionality gain outweighs the internationalization issues, imo.

@softwarerero
Copy link
Contributor Author

I could do the translations for #78. I have no idea if I could fix it in the pull request or if I should wait until you have merged it.

@softwarerero
Copy link
Contributor Author

You are right that it would be cleaner to change accounts-base directly. I will open a ticket over there and we'll see how they respond.

@queso
Copy link
Contributor

queso commented Feb 11, 2014

K. Happy to throw in on that, I love that we have such strong internationalization on this package :)

@softwarerero
Copy link
Contributor Author

MDG wants to deal with this post-1.0. I personally do not want to wait that long. Instead of the call above Session.set('entryError', (i18n err.reason.replace ".", "") || err.reason) we could call a helper function Session.set 'entryError', translate err.reason, so the day the MDG packages have translations we could just change that method to return its only parameter. That's the least intrusive way I could think of. To pull or not to pull then?

@queso
Copy link
Contributor

queso commented Feb 13, 2014

Ok, I can get behind that idea.

@softwarerero
Copy link
Contributor Author

I have added it to the open pull requests. In the future I have to find out how to assign unrelated commits to different pull requests.

@queso
Copy link
Contributor

queso commented Feb 17, 2014

This is related to PR #86

@queso
Copy link
Contributor

queso commented Feb 17, 2014

Merged #86.

@queso queso closed this as completed Feb 17, 2014
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

No branches or pull requests

2 participants