Eli Mesika has posted comments on this change.

Change subject: core, engine, webadmin: Retrieve Capabilities for each 
Architecture
......................................................................


Patch Set 8:

(3 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?
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
 archMap.put(version, isSupported(archCapabilitiesVerb, arch, version));
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 
ConfigValue as a parameter, this way you will not have to add method to support 
more values


-- 
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: 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