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