Roy Golan has posted comments on this change.

Change subject: core: OsInfo-related changes
......................................................................


Patch Set 23:

(5 comments)

http://gerrit.ovirt.org/#/c/30838/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java:

Line 179:             return vm.getGraphicsInfos().keySet();
Line 180:         } else {
Line 181:             List<VmDevice> graphicDevices =
Line 182:                     
DbFacade.getInstance().getVmDeviceDao().getVmDeviceByVmIdAndType(vm.getId(), 
VmDeviceGeneralType.GRAPHICS);
Line 183: 
Use EnumSet. it has lots of methods to help you here and is way more compact.

checkout EnumSet.of(...) etc.
Line 184:             Set<GraphicsType> graphicsTypes = new HashSet<>();
Line 185: 
Line 186:             for (VmDevice graphicDevice : graphicDevices) {
Line 187:                 GraphicsType type = 
GraphicsType.fromString(graphicDevice.getDevice());


http://gerrit.ovirt.org/#/c/30838/23/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DisplayType.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DisplayType.java:

Line 23:     public VmDeviceType getDefaultVmDeviceType() {
Line 24:         return defaultVmDeviceType;
Line 25:     }
Line 26: 
Line 27:     public static DisplayType fromString(String s) { // valueOf is 
case-sensitive
unless I'm missing something you should be good with
enum's native method valueOf(String string)

so

 DisplayType.valueOf("qxl")

should give you the qxl enum member.
Line 28:         for (DisplayType displayType: values()) {
Line 29:             if (displayType.toString().equalsIgnoreCase(s)) {
Line 30:                 return displayType;
Line 31:             }


http://gerrit.ovirt.org/#/c/30838/23/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java:

Line 91:      */
Line 92:     public int getMaximumRam(int osId, Version version);
Line 93: 
Line 94:     /**
Line 95:      * @return The supported display types for the given OS and 
cluster compatbility version
java doc needs update as well
Line 96:      */
Line 97:     public List<Pair<GraphicsType, DisplayType>> 
getGraphicsAndDisplays(int osId, Version version);
Line 98: 
Line 99:      /**


Line 96:      */
Line 97:     public List<Pair<GraphicsType, DisplayType>> 
getGraphicsAndDisplays(int osId, Version version);
Line 98: 
Line 99:      /**
Line 100:       * @return map (osId -> compatibility version -> display types 
list) for all OSs and
java doc needs update as well
Line 101:       * compatibility versions
Line 102:      */
Line 103:     public Map<Integer, Map<Version, List<Pair<GraphicsType, 
DisplayType>>>> getGraphicsAndDisplays();
Line 104: 


http://gerrit.ovirt.org/#/c/30838/23/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/OsRepositoryImpl.java
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/OsRepositoryImpl.java:

Line 334: 
Line 335:     private Pair<String, String> parseGraphicsAndDisplayPair(String 
displayAndGraphicsString) {
Line 336:         String[] displayAndGraphicsSplit = 
displayAndGraphicsString.split("/");
Line 337:         if (displayAndGraphicsSplit.length == 2) {
Line 338:             return new Pair<>(displayAndGraphicsSplit[0].trim(),
please use trimElements private method here.
Line 339:                     displayAndGraphicsSplit[1].trim());
Line 340:         }
Line 341:         return null;
Line 342:     }


-- 
To view, visit http://gerrit.ovirt.org/30838
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c7126308ab4e5e5418d804a8550a50e9994894
Gerrit-PatchSet: 23
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to