Ori Liel has posted comments on this change.

Change subject: engine: fix add watchdog
......................................................................


Patch Set 4:

(1 comment)

....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendWatchdogsResource.java
Line 67:         validateEnums(WatchDog.class, device);
Line 68:         watchdogParameters.setAction(getMapper(WatchdogAction.class, 
VmWatchdogAction.class).map(WatchdogAction.fromValue(StringUtils.upperCase(device.getAction())),
Line 69:                 null));
Line 70:         watchdogParameters.setModel(getMapper(WatchdogModel.class, 
VmWatchdogType.class).map(WatchdogModel.fromValue(StringUtils.upperCase(device.getModel())),
Line 71:                 null));
1) If you take an example from other enums, fromValue() includes 
.toUpperCase(), and that way clients don't have to activate it each time when 
calling the method: 

Example from VmType.java 
----------------------------------------
    public static VmType fromValue(String value) {
        try {
            return valueOf(value.toUpperCase());
        } catch (IllegalArgumentException e) {
            return null;
        }
    }

So it would be ideal if you can remove .toUpperCase() invocations from the 
client code and put them in the enum itself. 

2) I've just now noticed that WatchdogAction and WatchdogModel are in the wrong 
package: the should be alongside our other enums in restapi-definition project, 
package org.ovirt.engine.api.model. Can you please move them there?
Line 72:         watchdogParameters.setId(parentId);
Line 73:         watchdogParameters.setVm(isVm(parentId));
Line 74:         return watchdogParameters;
Line 75:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I95362fda69b8ec7c02893ba30cdf19be7eee5eac
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
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