Martin Mucha has posted comments on this change.

Change subject: engine: Ensure that all networks on NIC have QoS
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/34132/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java:

Line 252: validateNetworkQos
A)this method is rather long

B) it requires some time to identify what are separate code blocks and what are 
they supposed to do.

So I'd split this method into several ones:
* validity check of QosOverridden ifaces is one logical part not necessary in 
all branches of control flow. This could be extracted to separate method.

* calculating sets of ifaces names which are (not) used by some network and 
evaluation of these sets are two different calculations; it'd be more readable 
if they are separated and named.


Line 258: networkName == null
this condition can be extracted to variable with some good name explaining why 
we're not interested in such ifaces; it'd be more readable for those unfamiliar 
with related code. Name "ifaceWithoutNetworkAssigned" (is it that reason?) is 
stronger statement why we're not interested than "network name is not null".


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

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