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. > >> 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.
