Hi Shaopeng: This is looking good. I have a few nits and one comment of real substance (lang_COUNTRY selection).
Do you have a test for this class? http://codereview.appspot.com/109114/diff/3001/3002 File java/gadgets/src/main/java/org/apache/shindig/gadgets/render/OpenSocialI18NGadgetRewriter.java (right): http://codereview.appspot.com/109114/diff/3001/3002#newcode44 Line 44: } ctor not needed; @Inject will just find the class automatically. http://codereview.appspot.com/109114/diff/3001/3002#newcode52 Line 52: if (!gadget.getAllFeatures().contains("opensocial-i18n")) { nit: pull out into private static final String I18N_FEATURE_NAME = "opensocial-i18n"; http://codereview.appspot.com/109114/diff/3001/3002#newcode71 Line 71: String dataPath = "features/target/classes/features/i18n/data/"; change this to: res://features/i18n/data --> you'll get JS out of the runtime JARs rather than depending on the filesystem being set up just so. http://codereview.appspot.com/109114/diff/3001/3002#newcode73 Line 73: String localeName = locale.getLanguage(); For readability, I'd like to see this changed to: String language = locale.getLanguage(); String localeName = language; http://codereview.appspot.com/109114/diff/3001/3002#newcode75 Line 75: localeName = "en"; do you think it makes sense to introduce a "default" datafile with lang="all" that's a copy of the "en" constants? http://codereview.appspot.com/109114/diff/3001/3002#newcode83 Line 83: localeName += "_" + locale.getCountry(); I think you want getCountry().toUpperCase() and to suffix with ".js," else I don't see this ever succeeding http://codereview.appspot.com/109114/diff/3001/3002#newcode95 Line 95: inlineJs.append(dateTimeConstants.getContent()).append("\n").append(numberConstants.getContent());
100 char line
http://codereview.appspot.com/109114
