Code review comment for lp://staging/~dholbach/help-app/1423871

Revision history for this message
David Planella (dpm) wrote :

Looks good to me, thanks!

Just two comments:

- I initially suggested using a directory for each language to contain the generated files. In a conversation on IRC, Daniel mentioned that this presents an issue with CSS/JS relative imports' for the toolkit theme.
- On second thoughts, and if we cannot use the above, I'd suggest keeping the same filename for all files, but adding the language as a suffix, just as the Apache docs suggest to load translations [1]. That is:
 - index.en-us.html
 - index.de.html
 - get-in-touch.en-us.html (*)
 - get-in-touch.de.html

[1] http://httpd.apache.org/docs/2.2/content-negotiation.html
(*) Note the - vs. _ and lowercase. This is for converting gettext language codes to BCP-47 language codes, which are understood by the browser: http://www.rfc-editor.org/rfc/bcp/bcp47.txt

review: Needs Fixing

« Back to merge proposal