Einav Cohen has posted comments on this change. Change subject: engine,userportal,webadmin: enhance locale selection ......................................................................
Patch Set 5: (4 comments) patch looks ok, please see several inline comments. http://gerrit.ovirt.org/#/c/27973/5/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/UnsupportedLocaleHelper.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/UnsupportedLocaleHelper.java: Line 44: String[] localeKeys = locales.trim().split(" *, *"); //$NON-NLS-1$ Line 45: for (String localeKey: localeKeys) { Line 46: try { Line 47: //Check for valid locale. Line 48: org.apache.commons.lang.LocaleUtils.toLocale(localeKey); does this ^^^ support locale strings in the "xx-YY" format as well as the "xx_YY" format? (if not, I recommend to add a 'replace' here, just in case). Line 49: result.add(localeKey); Line 50: } catch (IllegalArgumentException iae) { Line 51: //The locale passed in is not valid, don't add it to the list. Line 52: } http://gerrit.ovirt.org/#/c/27973/5/backend/manager/tools/src/test/resources/engine-config.properties File backend/manager/tools/src/test/resources/engine-config.properties: Line 23: DomainName.alternateKey=domain_list Line 24: Line 25: MaxNumOfVmSockets.description=Max number of VM sockets. Version of this key is not general Line 26: Line 27: UnsupportedDisplayLocales.description=A comma separated list of locale keys that you want to display, this allows you to display some unsupported locales. ideally, we should incorporate here the list of unsupported locales (so the user would know what are the valid values for this config option) - not very easy to implement though (as the list of unsupported locales is itself a configuration value...) http://gerrit.ovirt.org/#/c/27973/5/frontend/webadmin/modules/userportal-gwtp/pom.xml File frontend/webadmin/modules/userportal-gwtp/pom.xml: Line 212: <configuration> Line 213: <property> Line 214: <name>gwt.availableLocales</name> Line 215: <value>{{ def bd=project.basedir; new File(db + 'backend/manager/modules/utils/src/main/resources/languages.properties').withInputStream { def prop = new Properties();prop.load(it);prop.keySet().sort().join(',')} }}</value> Line 216: </property> "new File(db + ...": should be "bd" instead of "db"? Line 217: </configuration> Line 218: </plugin> Line 219: </plugins> Line 220: </build> http://gerrit.ovirt.org/#/c/27973/5/frontend/webadmin/modules/webadmin/pom.xml File frontend/webadmin/modules/webadmin/pom.xml: Line 161: <version>0.2.5</version> Line 162: <configuration> Line 163: <property> Line 164: <name>gwt.availableLocales</name> Line 165: <value>{{ def bd=project.basedir; new File(db + 'backend/manager/modules/utils/src/main/resources/languages.properties').withInputStream { def prop = new Properties();prop.load(it);prop.keySet().sort().join(',')} }}</value> "new File(db + ...": should be "bd" instead of "db"? Line 166: </property> Line 167: </configuration> Line 168: </plugin> Line 169: <plugin> -- To view, visit http://gerrit.ovirt.org/27973 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b816694d6c34549360b189eb85c840688957bdb Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
