[node-patches] Change in ovirt-node[master]: Be even more thorough parsing /etc/ovirt-plugins.d/

fabiand at fedoraproject.org fabiand at fedoraproject.org
Fri Aug 16 09:55:53 UTC 2013


Fabian Deutsch has posted comments on this change.

Change subject: Be even more thorough parsing /etc/ovirt-plugins.d/
......................................................................


Patch Set 1:

(1 comment)

Basically I'm fine with this patch, as it seems that it fixes the problem.

But ... The function in question (to parse the ifnromations) is now deeply nested. I'd rather like to see being splitted up into smaller peaces into functions whith descriptive names and maybe also comments in the right places to explain the intention or rough workflow.
This would help reviewing and later bug fixing, once the memories about the solution fade a bit out of memory.

....................................................
File src/ovirt/node/setup/core/plugins_page.py
Line 39:     def name(self):
Line 40:         return "Plugins"
Line 41: 
Line 42:     def rank(self):
Line 43:         return -1
Could you explain the reason for a negative rank?
Line 44: 
Line 45:     def ui_content(self):
Line 46:         all_plugins = self.__list_of_plugins()
Line 47:         if all_plugins:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5fcbae07e2bd9d49c041bca13baa22bde3be07
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes



More information about the node-patches mailing list