Alon Bar-Lev has posted comments on this change.

Change subject: engine,userportal,webadmin: enhance locale selection
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.ovirt.org/#/c/27973/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java:

Line 1728:     UnsupportedDisplayedLocales,
Line 1729: 
Line 1730:     @TypeConverterAttribute(String.class)
Line 1731:     @DefaultValueAttribute("")
Line 1732:     UnsupportedLocalesFilter,
keep the space? :)
Line 1733:     Invalid


http://gerrit.ovirt.org/#/c/27973/1/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 30:                 if (!displayLocales.contains(unsupportedLocale)) {
Line 31:                     result.remove(unsupportedLocale);
Line 32:                 }
Line 33:             }
Line 34:         }
why can't we have:

 getLocalesKeys(unsupportediflter).removeAll(
     getLocalesKeys(unsupporteddisplay)
 )
Line 35:         Collections.sort(result);
Line 36:         return result;
Line 37:     }
Line 38: 


Line 45:         String locales = 
backend.runPublicQuery(VdcQueryType.GetConfigurationValue,
Line 46:                 new GetConfigurationValueParameters(configValues,
Line 47:                         
ConfigCommon.defaultConfigurationVersion)).getReturnValue();
Line 48:         if (locales != null && !locales.isEmpty()) {
Line 49:             String[] localeKeys = locales.split(","); //$NON-NLS-1$
locales.trim().split(" *, *") ?
Line 50:             for (String localeKey: localeKeys) {
Line 51:                 try {
Line 52:                     //Check for valid locale.
Line 53:                     
org.apache.commons.lang.LocaleUtils.toLocale(localeKey.trim());


http://gerrit.ovirt.org/#/c/27973/1/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java:

Line 199:         }
Line 200:         node.put(ConfigurationValues.UnsupportedLocalesFilter.name(),
Line 201:                 unSupportedLocales.substring(0, 
unSupportedLocales.length() - 1));
Line 202:         return node.toString();
Line 203:     }
I am glad we talked about this... but there must be a better way to do this 
without having to manually modify it with explicit fields every time. not for 
this patch of course..
Line 204: 
Line 205:     /**
Line 206:      * Get a JavaScript associative array string that define the 
branding messages.
Line 207:      * @param applicationName the application name.


-- 
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to