* Pradipta Kumar Banerjee <bpra...@in.ibm.com> [2012-10-02 06:41]:
> [snip]
> > Line 330: """
> > Line 331: arch = getPlatformArch()
> > One last nit. Unless we have another caller for getPlatformArch() I
> > suggest just doing:
> >
> > arch = platform.machine()
> Hi Ryan,
> I thought about it but at this point I'm not sure of other callers for
> getPlatformArch(). However I'm of the opinion of retaining the method.
> Is it a concern ?
Since you're introducting the call, no one else is using it yet. If in
the future getPlatformArch needs to be more complicated it can be added
at that time.
For now, let's minimize the amount of code we have to maintain. As a
reader of the code, if I see arch = getPlatformArch(), I need to then
look up the function. The current implementation is trivial enough not
to require a function; it adds nothing to readability to have a function
but it does increase the lines of code.
>
> >
> > and dropping the other function altogether.
> > Line 332: if arch == 'x86_64':
> > Line 333: out, err, ret = _logExec([EX_DMIDECODE, "-s",
> > "system-uuid"])
> > Line 334: out = '\n'.join( line for line in out.splitlines()
> > Line 335: if not line.startswith('#') )
> >
> >
> > --
> > To view, visit http://gerrit.ovirt.org/8094
> > To unsubscribe, visit http://gerrit.ovirt.org/settings
> >
> > Gerrit-MessageType: comment
> > Gerrit-Change-Id: I2d8767c52488ab8083123d0ec1e789d3857e2358
> > Gerrit-PatchSet: 4
> > Gerrit-Project: vdsm
> > Gerrit-Branch: master
> > Gerrit-Owner: Pradipta Banerjee <bpra...@in.ibm.com>
> > Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
> > Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
> > Gerrit-Reviewer: Pradipta Banerjee <bpra...@in.ibm.com>
> > Gerrit-Reviewer: Ryan Harper <ry...@us.ibm.com>
> >
>
>
> --
> Regards,
> Pradipta
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com
_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
