Dan Kenigsberg has posted comments on this change. Change subject: sos: Support SOS version 3 ......................................................................
Patch Set 4: Code-Review-1 (2 comments) We can make this even smaller (and better) http://gerrit.ovirt.org/#/c/25188/4/vdsm.spec.in File vdsm.spec.in: Line 241: Line 242: Requires: psmisc >= 22.6-15 Line 243: Requires: bridge-utils Line 244: Line 245: %if 0%{?fedora} >= 20 || 0%{?rhel} >= 7 This used to make sense with autoconf, but now, we should handle whatever sos version that we have installed. So there's no point in this change. Line 246: Requires: sos >= 3.1 Line 247: %else Line 248: Requires: sos < 3 Line 249: %endif http://gerrit.ovirt.org/#/c/25188/4/vdsm/sos/vdsm.py.in File vdsm/sos/vdsm.py.in: Line 68: self.addCopySpec(path) Line 69: Line 70: def setup(self): Line 71: # Make compatible com sos version >= 3 Line 72: if not sos.__version__.startswith("2."): It does not matter functionally, but performing these assignments on the class level are just nicer. Also, check for having the attribute is more pythonic than comparing versions. if not hasattrib(Base, 'addCopySpec'): addCopySpec = Base.add_copy_spec .... Line 73: self.addCopySpec = self.add_copy_spec Line 74: self.addCopySpecLimit = self.add_copy_spec_limit Line 75: self.collectExtOutput = self.add_cmd_output Line 76: self.getOption = self.get_option -- To view, visit http://gerrit.ovirt.org/25188 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52fbd9431506c5862cc762b4d3f0701ab5cf6a0b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
