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.
