Thanks Jan! I appreciate your patience. - Keith
Jan Damborsky wrote: > Hi Keith, > > please see my response in line. > > Thank you ! > Jan > > > Keith Mitchell wrote: >> Hi Jan, >> >> Responses below. I removed anything I have no further comments on. >> Thanks! >> >> - Keith >> >> Jan Damborsky wrote: >>> Hi Keith, >>> >>> thank you for your comments - I have accepted almost >>> all of them - please see below. >>> >>> I have updated webrev accordingly - it now contains only >>> changes related to your comments: >>> >>> http://cr.opensolaris.org/~dambi/python-24-26/ >>> >>> Thank you, >>> Jan >>> >>> >>>> >>>> >>>> 401-402: This line would be more readable if split up. >>> >>> I am not sure how you would like to have it split up. Could you >>> please elaborate more on this ? >>> >>> >> >> Something along the lines of the following would work: >> svc_address = service_list[svc_found_index].get_txt_rec() >> svc_address = svc_address.strip().split('aiwebserver=', 1)[1] >> svc_address = svc_address.split(',')[0] >> svc_address, svc_port = svc_address.split(':') >> >> It still looks somewhat messy, but it breaks up the chain of method >> calls so they're not wrapping multiple lines. > > I have changed this. > >> >>> >>>> 216-224: The 'platform' python module provides this functionality >>>> internally. Alternatively, os.uname() also exposes this data. >>>> Same comments for code in AICriteriaPlatform and AICriteriaCPU >>> >>> Well, I would prefer not to change this part at this point - also >>> I haven't found equivalent for 'uname -i' - do you happen to know >>> if there is one in 'platform' or 'os' module ? >> >> Ah, you're correct here, there doesn't appear to be a '-i' >> equivalent. The module non-public function >> "platform._syscmd_uname('-i')" appears to be a shortcut for running >> uname, but as it's a non-public interface, we can't rely on its >> stability. > > Agreed. Based on this, I didn't modify that part. > I have pushed all the changes into 2.6. gate. >
