Alon Bar-Lev has posted comments on this change. Change subject: kdump: Fix kexec-tools detection in node ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/29283/5/src/plugins/ovirt-host-deploy/kdump/packages.py File src/plugins/ovirt-host-deploy/kdump/packages.py: Line 111: from rpmUtils.miscutils import compareEVR Line 112: return compareEVR( Line 113: (None, node_version[0], node_version[1]), Line 114: (None, '0.2.0', None) Line 115: ) >= 0 no reason for having this as function, it is not reusable Line 116: Line 117: def _get_available_kexec_tools_versions(self): Line 118: versions = [] Line 119: if self.environment[odeploycons.VdsmEnv.OVIRT_NODE]: Line 144: package['release'], Line 145: ) Line 146: ) Line 147: Line 148: return versions why not return boolean if supported or not? you are mixing/splitting logics. if you have a function... all you ask is given this system do I support X? Line 149: Line 150: def _update_kdump_conf( Line 151: self, Line 152: content, -- To view, visit http://gerrit.ovirt.org/29283 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I136469c35e7fc11d3eb89ab5b4fc71395010f42b Gerrit-PatchSet: 5 Gerrit-Project: ovirt-host-deploy Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches