Edward Haas has posted comments on this change.

Change subject: net: edit bond detached from bridge but still attached to a vlan
......................................................................


Patch Set 8: Code-Review-1

(3 comments)

Small things...
I like the tests, very nice!

https://gerrit.ovirt.org/#/c/63723/8/tests/network/func_bond_basic_test.py
File tests/network/func_bond_basic_test.py:

PS8, Line 112: # BZ 1372798
Why is this needed? This info exists in the source control history.


PS8, Line 115: self.update_netinfo()
Can you add a comment on why it is needed?
In a week I will forget why it was done.


PS8, Line 124: self._test_detach_used_bond_from_bridge()
Calling this without a 'with'? What happens when it reaches the 'yield'?

Can't you just call it with 'pass'?


-- 
To view, visit https://gerrit.ovirt.org/63723
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I460cb08cf436b932e7d9592557a03d7b6fc36a0f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Edward Haas <edwa...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to