Mike Kolesnik has posted comments on this change.
Change subject: core: new base classes for GetAllImageListQuery
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractGetAllImagesListByStorageDomainIdQuery.java
Line 9: public abstract class AbstractGetAllImagesListByStorageDomainIdQuery<P
extends GetAllIsoImagesListParameters> extends AbstractGetAllImagesListQuery<P>
{
Maybe it's just me, but this inheritance is confusing since the base class is
searching by storage domain id too..
Perhaps the basic implementation should be to search by id from parameters, and
if an extending class wants to then it can override this behavior and return
the storage domain id some other way.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetAllIsoImagesListParametersBase.java
Line 4: public class GetAllIsoImagesListParametersBase extends
VdcQueryParametersBase {
Should be abstract?
Also is ISO in the name appropriate here?
--
To view, visit http://gerrit.ovirt.org/4147
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I58437e8985fad563cfe8b2fe059c5370166268e9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
