Alon Bar-Lev has posted comments on this change. Change subject: kdump: Fix version comparisons ......................................................................
Patch Set 3: (2 comments) http://gerrit.ovirt.org/#/c/29317/3/src/plugins/ovirt-host-deploy/kdump/packages.py File src/plugins/ovirt-host-deploy/kdump/packages.py: Line 96: elif major == '7': Line 97: min_version = self._create_version('2.0.4', '32.1') Line 98: Line 99: elif name == 'fedora': Line 100: min_version = self._create_version('2.0.4', '27') this helper function is just like writing: min_version = (None, '2.0.4', '27') why do you need it? it has no benefit... Line 101: Line 102: return min_version Line 103: Line 104: def _crashkernel_param_present(self): Line 175: cur_version = self._create_version( Line 176: package['version'], Line 177: package['release'], Line 178: ) Line 179: if compareEVR(cur_version, min_version) > -1: >=0 will be better or < 0 - the zero is the constant but why don't you use the compareVerOnly so you can keep the strings? Line 180: self.environment[odeploycons.KdumpEnv.SUPPORTED] = True Line 181: break Line 182: Line 183: self.logger.info( -- To view, visit http://gerrit.ovirt.org/29317 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ica14bd688c73a511ad8d7557231cff2e184830a9 Gerrit-PatchSet: 3 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: 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