Juan Hernandez has posted comments on this change. Change subject: restapi: Move Disk remove from collection to entity ......................................................................
Patch Set 1: (3 comments) https://gerrit.ovirt.org/#/c/42205/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/DisksResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/DisksResource.java: Line 1: package org.ovirt.engine.api.resource; > Shouldn't this patch include removing pefromRemove() from BackendDisksResou Yes, I forgot that. Line 2: Line 3: import javax.ws.rs.Consumes; Line 4: import javax.ws.rs.POST; Line 5: import javax.ws.rs.Path; https://gerrit.ovirt.org/#/c/42205/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java: Line 159: guid > Can combine the 2 above lines and use the constructor which accepts disk-id Done https://gerrit.ovirt.org/#/c/42205/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskResource.java: Line 71: @Override Line 72: public Response remove() { Line 73: get(); Line 74: RemoveDiskParameters parameters = new RemoveDiskParameters(); Line 75: parameters.setDiskId(guid); > Can combine 2 lines above and use constructor which receives disk-id I'll do that. However, take into account that the new code will be like this: return performAction(VdcActionType.RemoveDisk, new RemoveDiskParameters(asGuid(storageDomainId), guid)); Is that correct? Take your time to check it. It looks like it is correct, but it isn't, because the parameters are swapped. The tests will probably catch that, but. In general this is a disadvantage of having a method (or constructor) with multiple parameters of the same type. I believe that the alternative using the empty constructors and the setter is more readable and less error prone. Line 76: parameters.setStorageDomainId(asGuid(storageDomainId)); Line 77: return performAction(VdcActionType.RemoveDisk, parameters); Line 78: } -- To view, visit https://gerrit.ovirt.org/42205 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23e327b09bb3d8a87e49501d8980733b994c276c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
