Skip to content
This repository was archived by the owner on May 10, 2019. It is now read-only.

Conversation

@ozten
Copy link
Contributor

@ozten ozten commented Jan 12, 2012

This is a rebased version of the earlier pull request.

lloyd and others added 5 commits January 12, 2012 11:43
Adding i18n to wsapi. Split strings into client.po and messages.po.

Switched email from mustache to EJS templates.

Created fake locale db-LB for development and debugging.

Updated scripts.
See http://viewvc.svn.mozilla.org/vc?view=revision&revision=99911
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather one of these redirect to the other, so we stop bookmarking both.

@benadida
Copy link
Contributor

overall, this patch looks good, except for one fairly important thing I would strongly suggest tweaking.

The i18n overhead on a request appears to be non-negligible, just from eyeballing the amount of code that's run. But in fact we only need to do all of this stuff on two API calls that relate to sending email. Fortunately, every wsapi call is in its own file with its own metadata now. So we can easily add

exports.i18n = true;

in those wsapi calls that need it, and your middleware can bail early if i18n is not needed for a given call.

@ozten
Copy link
Contributor Author

ozten commented Jan 17, 2012

I'm waiting for r+ before landing this on dev.

I'll merge again to keep the PR merge button green.

ozten added 5 commits January 17, 2012 09:29
…alization_v2

Conflicts:
	lib/configuration.js
	resources/static/dialog/views/pick_email.ejs
	resources/static/shared/renderer.js
…alization_v2

Conflicts:
	resources/views/dialog_layout.ejs
	resources/views/test.ejs
	scripts/compress.sh
…alization_v2

Conflicts:
	lib/configuration.js
@lloyd
Copy link
Contributor

lloyd commented Jan 18, 2012

@ozten and I consulted gerv, and now understand that the final outstanding issue is removing the GPL licensed implementation of gettext(), as it would mean we couldn't release browserid under MPL without violating his license.

Specifically, this library is the issue: https://github.com/mozilla/browserid/blob/a725d667e4ce537404006037d1d72a7c5fd6a1d7/resources/static/shared/gettext.js

Choose your own adventure: rewrite, choose another, or contact the author?

@ozten
Copy link
Contributor Author

ozten commented Jan 18, 2012

Lame. Sorry about that and thanks @lloyd for spotting this legal bug.

There are several client side gettext to choose from. I'll remove and stub out, then replace with a functioning gettext.

@ozten
Copy link
Contributor Author

ozten commented Jan 18, 2012

Following up on other new code:

LGPL libraries:
node-gettext
lib-iconv

ozten added a commit that referenced this pull request Jan 18, 2012
Bug706572 localization v2 r=benadida,stomlinson,lloyd
@ozten ozten merged commit 9e12c06 into dev Jan 18, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants