Hi Karen -
thanks for the review...see below.
ginnie
On 11/24/10 13:26, Karen Tung wrote:
Hi Ginnie,
I have a few comments/questions:
1) line 90: You are just removing the line feed or carriage return
here. Would it
affect readability of the resulted line. For example, if I have this:
Installing SUNWcsu<line feed>
to /a
It will look OK with the line feed. However, if you just remove the
line feed,
then, it will look like:
Install SUNWcsuto /a
Should the line feed/carriage return be replaced by a space to maintain
readability?
I do have to include the line feed/carriage return in order to consume
the characters that are passed by the print statement, which is the main
reason for this code.
John Fischer's suggestion, I believe will address this issue. Let me
follow up on this once I've had a chance to look at it a bit more.
2) Line 86-92: Do we really need to use "tmp"? Since we are not saving
self.data for future use, why not just do the checking for the special
chars
and remove them in place?
Actually, I don't think I do. I'll fix that.
3) 451, 459-460: Instead of assuming that the previous value for
sys.stdout and sys.stderr
is sys.__stdout__ and sys.__stderr, I think it is better to save the old
values before setting in line 451, and then, restore them to whatever
the old value might be.
Some apps might have set stdout and stderr to special values.
4) 491, 500-501: same comment as above.
ok. Let me look at this a bit more.
Thanks,
--Karen
On 11/24/10 11:39 AM, Ginnie Wray wrote:
Hi -
I would like to request a code review for the following:
Detailed output when reporting package transfer needed/remove unneeded
output
http://monaco.sfbay.sun.com/detail.jsf?cr=7001612
Webrev is located at:
http://cr.opensolaris.org/~ginnie/7001612/
I tested slim_source nose tests and also ran those in the cud_dc gate
for dc.
thanks
ginnie
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
