John Fischer wrote: > All, > > I have finally updated the webrev for the list changes. I believe > that I have addressed the concerns that were related to the list > and not something missing elsewhere. When these other issues are > resolved I will update list. I will follow up on Monday by filing > bugs against these other issues. > > The code was checked by 'hg nits' and pylint using the install.pylintrc > file sent out by Jean. hg nits did not find anything to correct. The > only complaints pylint found were: > > C0302 - Too many lines in module (%s) > E0602 - Undefined variable '_' -- defined via gettext __init__ > > The new webrev is at: > > http://cr.opensolaris.org/~johnfisc/list >
installadm.h. 54: nit, align expansion with neighboring lines delete_service.py 187: the design for the INETd_CONF functionality is flawed (sigh), but that's not your fault; I've filed bug 13123 to cover this issue. 205, 218: stylistically, I'd think it makes more sense to put the comment inside the except block than outside. 222ff: This logic isn't quite the same as what in.tftpd uses when -s is specified (which is the normal and recommended case). If -s is used and the directory doesn't exist the the tftp service should go to maintenance. I think you can simplify on this basis. list.py 211: s/ot/to/ 246: seems that you should just use socket.gethostyname_ex() here 311: don't grok this comment... 316: should we be using getfqdn() instead? 322: this comment could be construed that you're creating a directory, might rephrase 379: this comment seems incorrect; /etc/netboot doesn't appear to be used anywhere here (correctly) 560: how would this condition be the case? Should we do something stronger than just skip it? 916: Seems like we can get some sort of traceback out of this operation, as I don't see except clauses that will catch a failure anywhere further up the chain. Dave
