Thanks for the feedback Jan. - Keith
Jan Damborsky wrote: > 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 >
