Vitor de Lima has posted comments on this change. Change subject: core, engine, webadmin: Retrieve Capabilities for each Architecture ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/23238/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetArchitectureCapabilitiesQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetArchitectureCapabilitiesQuery.java: Line 25: } Line 26: Line 27: private synchronized Map<ArchitectureType, Map<Version, Boolean>> getMap(ArchCapabilitiesVerb archCapabilitiesVerb) { Line 28: Line 29: if (verbMap.containsKey(archCapabilitiesVerb)) { > do we have to handle NULL argument passed to this method? I have handled this, just in case... Line 30: return verbMap.get(archCapabilitiesVerb); Line 31: } else { Line 32: Map<ArchitectureType, Map<Version, Boolean>> supportMap = Line 33: new EnumMap<ArchitectureType, Map<Version, Boolean>>(ArchitectureType.class); Line 37: Line 38: for (Version version : Version.ALL) { Line 39: boolean isSupported = isSupported(archCapabilitiesVerb, arch, version); Line 40: Line 41: archMap.put(version, isSupported); > Can be Done Line 42: } Line 43: Line 44: supportMap.put(arch, archMap); Line 45: } http://gerrit.ovirt.org/#/c/23238/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java: Line 257: * @return Line 258: */ Line 259: public static boolean isSuspendSupportedByArchitecture(ArchitectureType architecture, Version version) { Line 260: return supportedInConfig(ConfigValues.IsSuspendSupported, version, architecture); Line 261: } > I think all 3 should be replaced with one single method that gets also the Are you sure? This would go against the standard imposed on the rest of this class (which has one method per Feature). http://gerrit.ovirt.org/#/c/23238/8/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 1072: <xs:appinfo> Line 1073: <jaxb:property name="Capabilities"/> Line 1074: </xs:appinfo> Line 1075: </xs:annotation> Line 1076: <xs:element ref="architecture_feature" maxOccurs="unbounded"/> > To be consistent this should be "architecture_capability". Done Line 1077: </xs:sequence> Line 1078: </xs:complexType> Line 1079: Line 1080: <!-- Common to all resources --> -- To view, visit http://gerrit.ovirt.org/23238 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied1a97f5bdd42382f0dc832a3fe3abd56d3c89dd Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Laszlo Hornyak <[email protected]> Gerrit-Reviewer: Leonardo Bianconi <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: Vitor de Lima <[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
