Allon Mureinik has posted comments on this change.

Change subject: core: Add procedure to get active Hosts per Data Center
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(2 inline comments)

....................................................
File backend/manager/dbscripts/vds_sp.sql
Line 833: CREATE OR REPLACE FUNCTION GetAllByPoolIdAndStatus(v_storage_pool_id 
UUID, v_vds_status int) RETURNS SETOF vds
Missed it in the previous review somehow, sorry.

psql functions don't have packages, so unless i'm looking at it in this file's 
context, it's hard to understand what "GetAllByPoolIdAndStatus" does - get all 
WHAT?
Consider renaming ti GetAllVdsByPoolIdAndStatus.

....................................................
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java
Line 18:     private final Guid STORAGE_POOL_ID = new 
Guid("6d849ebf-755f-4552-ad09-9a090cda105d");
The convention is to store there constants in the FixturesTool class.

If you don't want to put it there, at least make this variable final.

--
To view, visit http://gerrit.ovirt.org/6144
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia88a820dc87e9b2178a4ef99a2e22762d1824bfb
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@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

Reply via email to