Hi Keith,
On 12/24/09 06:14 PM, Keith Mitchell wrote: >> >> text-install.py >> --------------- >> 35 import subprocess >> >> Since only call() is used from 'subprocess' Python module, >> could we just import this one and simplify to call to it - e.g. >> >> 35 import subprocess >> -> >> 35 from subprocess import call >> >> 240,251 subprocess.call >> -> >> 240,251 call >> >> Ditto for 'platform' module - only processor() is consumed: >> >> 33 import platform >> -> >> 33 from platform import processor > > > I prefer the first form, as it makes it more obvious where the > 'call()' and 'processor()' functions are from in this case. I'm not > tied to either form though. That makes sense - I will let you decide what form you would like to use. > >> >> 101 if logname is not None: >> -> >> 101 if logname: > > I'm checking to make sure that the caller passed in a parameter (i.e., > that we're not using the default parameter). In this particular case, > either would work identically, since logname "should" be a string > object, but I want to be consistent with other functions where I > follow the same format. Thank you for clarifying this. Based on this, I am fine with the original code. >> >> >> base_screen.c >> ------------- >> 58,59,60 - just a nit - please add space between 'Installer' word >> and subsequent question mark > > May I ask why? Question marks should be directly after the word. You are right - please discard that comment. Thank you, Jan
