Ayal Baron has posted comments on this change.
Change subject: core: Added GetManagementInterfaceAddressByVdsId query
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetManagementInterfaceAddressByVdsIdQuery.java
Line 6: public class GetManagementInterfaceAddressByVdsIdQuery<P extends
GetManagementInterfaceAddressByVdsIdParameters> extends QueriesCommandBase<P> {
s/Management/mgmt/
s/Interface/iface/
s/Address/Addr/
to get something that doesn't occupy the entire line yet is identified as the
same thing:
GetMgmtIfaceAddrByVdsIdQuery
(personally I'd also change all queries just to start with Q or something by
convention to shorten it even more, and change VdsId to Vds but that is another
story).
Also, a new class for simply passing VdsId? are you serious?
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/InterfaceDAODbFacadeImpl.java
Line 139: .addValue("vds_id", id).addValue("user_id",
userID).addValue("is_filtered", isFiltered);
Please split this up into 3 separate distinct calls.
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/InterfaceDAOTest.java
Line 258: public void
testGetManagedInterfaceForVdsByVdsIdWithPermissionsForUnpriviligedUser() {
s/unbearablyLongName/testGetManagedIfaceNonPrivileged/
?
Line 267: public void
testGetManagedInterfaceForVdsByVdsIdWithPermissionsForPriviligedUser() {
s/unbearablyLongName/testGetManagedIfacePrivileged/
?
Line 281: private void
assertCorrectGetManagedInterfaceForVdsResult(VdsNetworkInterface result) {
Is' interface' actually 'nic'? if so, why not just call it that all over?
it's clearer as to the intent and shorter (win win)
--
To view, visit http://gerrit.ovirt.org/4854
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6d855734c6cdec937de6ec91d1c80a23333eec8
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
