Arik Hadas has posted comments on this change.
Change subject: restapi,engine: fix usb mapping on update vm call
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
Line 422: VDSGroup vdsGroup,
Line 423: List<String> messages) {
Line 424: boolean retVal = true;
Line 425: if (usbPolicy == null) {
Line 426:
messages.add(VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_USB_POLICY.toString());
The tradeoff between the two options is clear:
the advantages of calling name():
1. since we currently need the exact field name, it's better to call this
method which returns the exact field name and it can't be changed (because it's
final)
2. it would allow us to make future use of toString method of those enum values
the advantages of calling toString():
1. this class is heavily used in the system, many people used it before and are
used to call its toString method. there are likely to be mistakes by people
that will keep calling the toString method in future changes that we'll need to
prevent
2. it would be easier to change the keys of the errors in the future (which are
currently the exact name of the values in this enum) because the toString
method can be overridden
3. it won't require changing many places in the code
it's a very small issue which is really more a matter of taste, and it would
work either way.
I'm checking whether it is really necessary to raise this error - if we don't
need to raise an error this code is irrelevant, otherwise I wouldn't mind
implementing in any of the suggested ways
Line 427: retVal = false;
Line 428: } else if (UsbPolicy.ENABLED_NATIVE.equals(usbPolicy)) {
Line 429: if (!Config.<Boolean>
GetValue(ConfigValues.NativeUSBEnabled, vdsGroup.getcompatibility_version()
Line 430: .getValue())) {
--
To view, visit http://gerrit.ovirt.org/9792
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If74bf6ede2fc0f932db8dfcd7951b6bcf90b7744
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
