Tomas Jelinek has posted comments on this change. Change subject: frontend: Add NONE to GraphicsTypes enum & clean code ......................................................................
Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/38171/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java: Line 877: } Line 878: Line 879: public static GraphicsTypes fromGraphicsTypes(Collection<GraphicsType> types) { Line 880: for (GraphicsTypes myTypes : values()) { Line 881: if (myTypes.getBackingGraphicsTypes().equals(types)) { This is not completely correct, because: - the myTypes.getBackingGraphicsTypes() is a HashSet - the HashSet inherits the equals() from AbstractSet - AbstractSet.equals(arg) returns false if the 'arg' is not an instance of a Set - the fromGraphicsTypes accepts any collection So, if you passed e.g. an Arrays.asList(...) to it it would return false all the time regardless of the content. So please just change the declaration from fromGraphicsTypes(Collection<GraphicsType> types) { to fromGraphicsTypes(Set<GraphicsType> types) { Line 882: return myTypes; Line 883: } Line 884: } Line 885: -- To view, visit https://gerrit.ovirt.org/38171 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib69e8d687495bddabff28fe6a061c8118ff1983d Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
