Michal Skrivanek has posted comments on this change. Change subject: ppc64le: fix cpuinfo caps parsing ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/39059/1/vdsm/caps.py File vdsm/caps.py: Line 143: class Architecture: Line 144: X86_64 = 'x86_64' Line 145: PPC64 = 'ppc64' Line 146: PPC64LE = 'ppc64le' Line 147: POWER = (PPC64, PPC64LE) > Defining "enumerables" of two different types does not feel right. It requi I actually like it a bit better. is_power() is too...I don't know..looks to me too much as a function which is supposed to do something more. Whereas having something clear in capital letters stands out clearly - which I think is appropriate here as it is an exception for different architecture(s). So if possible it should stand out the same way as X86_64 does not that I mind, it's just a personal taste... Line 148: Line 149: Line 150: class CpuInfo(object): Line 151: def __init__(self, cpuinfo='/proc/cpuinfo'): Line 193: Line 194: class CpuTopology(object): Line 195: def __init__(self, capabilities=None): Line 196: Line 197: if platform.machine() == Architecture.PPC64: > can you explain why a future patch? It seems logically related to this patc it's removed by https://gerrit.ovirt.org/#/c/33872/ Line 198: from ppc64HardwareInfo import \ Line 199: getCpuTopology as getPPC64CpuTopology Line 200: self._topology = getPPC64CpuTopology(capabilities) Line 201: else: -- To view, visit https://gerrit.ovirt.org/39059 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I042e817e4f97866be3a762efb3cf7afdd3ecee7f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Madhu Pavan <k...@linux.vnet.ibm.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
