Vojtech Szocs has posted comments on this change.

Change subject: engine,userportal,webadmin: configurable unsupported locales
......................................................................


Patch Set 9:

(1 comment)

Just one small comment in VisibleLocalesInfoData.java - otherwise, looks good 
to me.

http://gerrit.ovirt.org/#/c/27973/9/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/VisibleLocalesInfoData.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/VisibleLocalesInfoData.java:

Line 49:         List<String> hiddenList = new 
ArrayList<String>(allLocaleNames);
Line 50:         hiddenList.removeAll(visibleLocales);
Line 51:         result.removeAll(hiddenList);
Line 52:         Collections.sort(result);
Line 53:         return result.toArray(new String[result.size()]);
Minor thing, IMHO working with Java arrays is not very comfortable (compared to 
collections) so we could just return List<String> here and in 
AbstractLoginFormView we could change "String[] localeNames" to "List<String> 
localeNames".

When using arrays, I'd personally prefer:

 result.toArray(new String[0]); // new array is allocated, no need to worry 
about size
Line 54:     }


-- 
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: 9
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: Yair Zaslavsky <[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