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