Moti Asayag has posted comments on this change.

Change subject: engine: filter external network labels out
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.ovirt.org/#/c/26001/1//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2014-03-23 13:00:13 +0200
Line 6: 
Line 7: engine: filter external network labels out
Line 8: 
Line 9: Network labels for RHEV and for Neutron should not be mixed when 
creating a network and attaching a label to it.
1. please reformat the commit message according to the template restrictions 
(Longer description using lines' length under 72 chars.)

2. RHEV is a downstream product, please use ovirt-engine instead.

3. s/attaching a label to it/labelling it
Line 10: For RHEV network only RHEV labels should be presented for newly 
created network and for Neutron network only Neutron labels.
Line 11: 
Line 12: Change-Id: I3a84f42b6c14527ff7a1805a61d131dbcb8ad2ca
Line 13: Bug-Url: https://bugzilla.redhat.com/1077132


Line 6: 
Line 7: engine: filter external network labels out
Line 8: 
Line 9: Network labels for RHEV and for Neutron should not be mixed when 
creating a network and attaching a label to it.
Line 10: For RHEV network only RHEV labels should be presented for newly 
created network and for Neutron network only Neutron labels.
the second part of this sentence mislead, since this patch didn't introduce any 
change in regards to neutron network labels. Please remove it (unless you wish 
to fix another bug which relates to it ;-))
Line 11: 
Line 12: Change-Id: I3a84f42b6c14527ff7a1805a61d131dbcb8ad2ca
Line 13: Bug-Url: https://bugzilla.redhat.com/1077132


http://gerrit.ovirt.org/#/c/26001/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDao.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDao.java:

Line 105:     List<Network> getAllForProvider(Guid id);
Line 106: 
Line 107:     /**
Line 108:      * Retrieves all network labels defined on internal networks in a 
specific data-center
Line 109:      * 
trailing white spaces will cause checkstyle failure.
Line 110:      * @param id
Line 111:      *            the data-center id
Line 112:      * @return all labels defined for the data-center's networks
Line 113:      */


Line 108:      * Retrieves all network labels defined on internal networks in a 
specific data-center
Line 109:      * 
Line 110:      * @param id
Line 111:      *            the data-center id
Line 112:      * @return all labels defined for the data-center's networks
perhaps mention "data-center's internal networks" here as well ?
Line 113:      */
Line 114:     Set<String> getInternalNetworkLabelsForDataCenter(Guid id);
Line 115: 
Line 116:     /**


http://gerrit.ovirt.org/#/c/26001/1/backend/manager/modules/dal/src/test/resources/fixtures.xml
File backend/manager/modules/dal/src/test/resources/fixtures.xml:

Line 442:         <column>id</column>
Line 443:         <column>name</column>
Line 444:         <column>description</column>
Line 445:         <column>is_local</column>
Line 446:         <column>status</column>
usually we don't introduce changes unrelated to the patch.

This for example could come as a solely patch.
Line 447:         <column>master_domain_version</column>
Line 448:         <column>spm_vds_id</column>
Line 449:         <column>compatibility_version</column>
Line 450:         <column>_create_date</column>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a84f42b6c14527ff7a1805a61d131dbcb8ad2ca
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[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

Reply via email to