Ondřej Svoboda has posted comments on this change.

Change subject: configurators: move disable_ipv6 code to a common function
......................................................................


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/46984/3/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 47:     from ovirt.node.utils import fs as node_fs
Line 48: 
Line 49: from . import Configurator, getEthtoolOpts, update_ipv6_sysctl
Line 50: from . import dhclient
Line 51: from . import libvirt
> why not
dhclient and libvirt are modules, while the rest is contained in __init__.py

Even though I came up with this split myself, we may find some inspiration 
elsewhere. git-review follows (probably as well as other OpenStack projects) a 
convention "do not import more than one module per line".

While there is also a more rigid imperative of "not importing objects, just 
modules", I think this change goes the right way still.

https://github.com/openstack-infra/git-review/blob/master/HACKING.rst
Line 52: from ..errors import ConfigNetworkError, ERR_FAILED_IFUP
Line 53: from ..models import Nic, Bridge, IPv4, IPv6
Line 54: from ..sourceroute import StaticSourceRoute, DynamicSourceRoute
Line 55: from ..utils import remove_custom_bond_option


https://gerrit.ovirt.org/#/c/46984/3/vdsm/network/configurators/iproute2.py
File vdsm/network/configurators/iproute2.py:

Line 28: from vdsm.utils import CommandPath
Line 29: from vdsm.utils import execCmd
Line 30: 
Line 31: from . import Configurator, getEthtoolOpts, runDhclient, 
update_ipv6_sysctl
Line 32: from . import libvirt
> and again! what's you motive?
Answered on ifcfg.py (split modules from objects).
Line 33: from .dhclient import DhcpClient
Line 34: from ..errors import ConfigNetworkError, ERR_FAILED_IFUP, 
ERR_FAILED_IFDOWN
Line 35: from ..models import Nic
Line 36: from ..sourceroute import DynamicSourceRoute


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I39d39f10cc2e7f593bb4ba3c65c0e04ff08715ee
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to