[node-patches] Change in ovirt-node[master]: Drop the MAC from the network page if systemd

fabiand at redhat.com fabiand at redhat.com
Thu Feb 12 13:57:46 UTC 2015


Fabian Deutsch has posted comments on this change.

Change subject: Drop the MAC from the network page if systemd
......................................................................


Patch Set 4:

(1 comment)

Just a nti, then fine

http://gerrit.ovirt.org/#/c/33345/4/src/ovirt/node/setup/core/network_page.py
File src/ovirt/node/setup/core/network_page.py:

Line 49:                                        header,
Line 50:                                        self._get_nics(filter_configured),
Line 51:                                        height=height, multi=multi),
Line 52: 
Line 53:     def __build_header(self, managed, systemd, multi):
Please use with_mac instead fo systemd here.

The reasoning is: That the syetmd knowledge should stay outside the function, or inside. (I preferre the outside). But it does not mae sense to have the systemd knowledge outside, and then pass it inside to the function (with the systemd variable).

It is better to transform the knowled, or do the logic outside the function i.e.:

    have_macs = if systemd then False else True
    do_wahtever(with_macs=have_macs)
Line 54:         if not managed:
Line 55:             if not systemd:
Line 56:                 header = "Device  Status       Model         MAC Address"
Line 57:             else:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7358fd2d479b0d257a4558514f4ead680eed4b19
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes



More information about the node-patches mailing list