Drew,
Thanks for the review. I'm commenting on those things that Dermot
left for me to handle.
Thanks,
John
usr/src/cmd/gui-install/src/progress_screen.py
----------------------------------------------
156-174: I had no idea the TextFile class existed. Why the heck did
Python stick that class in the distutils module instead of somewhere
more universal?!
I think it reads easier if you use the readlines() call though instead
of a readline() while loop:
urlimage_dict = dict()
image_fh = TextFile(....)
for line in image_fh.readlines():
if "=" in line:
filename, sep, url = line.partition("=")
filename = os.path.join(path, filename)
urlimage[filename] = url
return urlimage_dict
leaving for John.
Corrected as suggested.
263-282: There's an entire in-line function setup to create a
software node
for CPIO that's used only once and only inside this class method.
That tells me there's no reason for an in-line function here.
leaving for John
This code is snarfed from the text_installer. I was going to suggest that
several pieces of this section of code be moved elsewhere that could then
be used by both GUI install and Text install. Karen and I are talking about
doing this type of thing later. Until then I'll move it out of a function.
usr/src/cmd/gui-install/src/user_screen.py
------------------------------------------
159, 171-172: You default data=None but then try to split it out. Is
it possible to traceback here because of that? If this wasn't a pygtk
callback method, I would think you'd have to add special handling for
it but I'm not sure if pygtk takes care of it for you or not....
leaving for John
Corrected with:
if data is None:
return
user_password = data[0]
check_password = data[1]
283-285: We could use str.translate here to make this a lot more
readable:
import string # depricated, but that's ok
# construct a map of viable characters for a hostname
viable = string.letters + string.digits + "-_."
if " " in hostname or hostname.translate(None, viable):
return False
return True
leaving for John
Changed as suggested.
275-289: Again, we have a function used only once. Can we remove the
function call part of this?
leaving for John
I would rather leave in the function to keep the code consistent with the
other validation for the user. Also I am hoping that it will be
possibly removed
if and when the system-configuration stuff does hostname validation.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
