Dan Kenigsberg has posted comments on this change. Change subject: ppc64: unable to list online logical CPUs ......................................................................
Patch Set 2: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/38983/2/vdsm/ppc64HardwareInfo.py File vdsm/ppc64HardwareInfo.py: Line 58: Line 59: return infoStructure Line 60: Line 61: Line 62: def parseCpus(value): this can (and should) be private. Please name new functions according to pep8. maybe _parse_cpu_list() Please add an explicit unit test for this function. I'd like to see broader tests. Line 63: onlineCpus = [] Line 64: if value.find(',') == -1 and value.find('-') == -1: Line 65: onlineCpus.append(str(value)) Line 66: elif value.find(',') == -1: Line 61: Line 62: def parseCpus(value): Line 63: onlineCpus = [] Line 64: if value.find(',') == -1 and value.find('-') == -1: Line 65: onlineCpus.append(str(value)) I still do not understand the str() conversion. What is value exactly? why does it need a conversion at all? Line 66: elif value.find(',') == -1: Line 67: cpuindex = value.split('-') Line 68: for cpu in range(int(cpuindex[0]), int(cpuindex[1])+1): Line 69: onlineCpus.append(str(cpu)) Line 64: if value.find(',') == -1 and value.find('-') == -1: Line 65: onlineCpus.append(str(value)) Line 66: elif value.find(',') == -1: Line 67: cpuindex = value.split('-') Line 68: for cpu in range(int(cpuindex[0]), int(cpuindex[1])+1): style: use spaces around operators like "+". Line 69: onlineCpus.append(str(cpu)) Line 70: else: Line 71: for cpusets in value.split(','): Line 72: if cpusets.find('-') == -1: -- To view, visit https://gerrit.ovirt.org/38983 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5c4bdbc00217516ba3b7e361ab28e409c507baa Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Madhu Pavan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Madhu Pavan <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[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
