Juan Hernandez has posted comments on this change. Change subject: rest: delete a disk from disks collection of a snapshot ......................................................................
Patch Set 4: (2 comments) http://gerrit.ovirt.org/#/c/26328/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotElementsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotElementsResource.java: Line 5: Line 6: import javax.ws.rs.core.Response; Line 7: Line 8: public abstract class BackendSnapshotElementsResource<R extends BaseResource> Line 9: extends AbstractBackendCollectionResource<R, Snapshot> { > Do you mean using AbstractBackendCollectionResource directly? It means we'l It took me a few minutes to understand what is the purpose of this class, so I don't think it makes things clearer. Also, the way "performRemove" is implemented (throwing an exception) means that the "remove" operation doesn't make sense for some of the subclasses, which in turn means that some subclasses aren't really specialized versions of this one. I'm not in favour of these abuses of inheritance. However, I don't have a strong argument against this, so do as you see better. Line 10: Line 11: protected BackendSnapshotResource parent; Line 12: Line 13: protected BackendSnapshotElementsResource(Class<R> modelType, Class<Snapshot> entityType, String... subCollections) { http://gerrit.ovirt.org/#/c/26328/4/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/FeaturesHelper.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/FeaturesHelper.java: Line 85: } Line 86: Line 87: private void removeDiskSnapshot(Features features) { Line 88: Feature feature = new Feature(); Line 89: feature.setName("Remove disk snapshot"); > The functionality is actually removing a snapshot of a disk... but I can ch That makes sense, please do. Line 90: feature.setDescription("Ability to remove a disk from a VM snapshot"); Line 91: features.getFeature().add(feature); Line 92: } Line 93: -- To view, visit http://gerrit.ovirt.org/26328 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic7fb780516ba715d87e59e98651e9d03dad15845 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
