yola / gulp-static-i18n

Gulp plugin for internationalization of static assets.
MIT License
5 stars 8 forks source link

Add language code prefixing to json translator #15

Closed euwest closed 9 years ago

euwest commented 9 years ago

fixes https://github.com/yola/gulp-static-i18n/issues/13

passing the urlKeys option(an array of keys at which urls are stored on json) will cause urls at those keys to be prefixed with a language code(e.g. /es/, /zh-cn/ et al.) upon translation.

beck commented 9 years ago

This looks good. The one thing you're missing is locale > lang conversation for the prefixing.

A bit of critique about your commits: Do not commit your tests & functional changes separately.

Ideally each commit contains:

It is good practice to commit code often and push every so often to protect yourself when your machine rises up against you, but before a pull happens, rebase it and make it pretty. Make each commit "matter" and have isolated purpose.

For this specific pull I would expect to see two commits:

  1. Cleanup array handling
  2. Add language code prefixing to json translator
euwest commented 9 years ago

The one thing you're missing is locale > lang conversation for the prefixing.

are you talking about pt-br => pt_BR | zh-cn => zh_CN? since this is public, it seems odd to handle our very specific language-code issues here. i guess we could include support for a langMap option that would look something like:

var langMap = {
  "en": "",
  "pt-br": "pt_BR",
  "zh-cn": "zh_CN"
  ...
};
beck commented 9 years ago

No no, write something like this

beck commented 9 years ago

And keep english prefixing

beck commented 9 years ago

:+1: