On 08/16/10 04:13 PM, Ginnie Wray wrote:
Hi Karen -
Thanks for the feedback.
ginnie
On 08/16/10 12:46 PM, Karen Tung wrote:
Hi Ginnie,
I got a few comments on logger.py:
- lines 253-255: Why do you want to catch the assertion and print
it? Why not let the
caller handle the problem, since they provided an invalid progress
value.
Would you like me to remove the traceback.print_exc()?
I have to have to manage the failure for the C bridge code , or I get
a false success when
the function fails.
I believe we resolved this during our meeting this morning.
- line 267-269: these 3 lines of code will not be correct if the
destination provided
is a file, instead of a directory.
I've modified this due to some other feedback. However, the design doc
stipulated that the
destination would be a directory and not a file.
Ah, OK, I was not aware that the destination would be a directory.
Since you are just
moving the default log, which is just one file, would it make sense to
allow the destination to be a file name as well?
Thanks,
--Karen
- line 280: isn't it better to check "if handler.baseFilename not in
close_log_list"?
I'll change this.
Thanks,
--Karen
On 08/09/10 16:11, Ginnie Wray wrote:
Hi -
I've incorporated the changes from the last code review.
I would like feedback for these changes by Monday Aug 16th.
Code is located at:
http://cr.opensolaris.org/~ginnie/logger0809/
I've also included output from the nose tests:
bash-4.0$ ./slim_test lib/install_logging_pymod/test --with-coverage
#1 Ensure that FileHandlers can be added to a logger ... ok
#2 Ensure that StreamHandlers can be added to a logger ... ok
#3 Ensure that InstallLogger close works ... ok
#4 Ensure default_log is created and uses the default format. ... ok
#5 Ensure that only one InstallLogger is created for Install Logger
... ok
#6 Ensure that default format is set for the InstallLogger ... ok
#7 Ensure that a log message below the designated level does not log
... ok
#8 Ensure that default log is returned with default_log method ... ok
#9 Ensure that logger name is returned correctly ... ok
#10 Ensure that logger instance is from InstallLogger ... ok
#11 Ensure that debug log messages are logged to the log file ... ok
#12 Ensure that info log messages are logged to the log file ... ok
#13 Ensure that warning log messages are logged to the log file ... ok
#14 Ensure that default log transfers to destination ... ok
#15 Ensure that source file transfers to desired destination ... ok
#16 Test the message returned by ProgressLogRecord get_message ... ok
#17 Test that a second progress handler can be added ... ok
#18 'Tests that progress is reported to the default log ... ok
#19 Test that progress is reported to the progress receiver ... ok
#20 Tests that report_progress fails is value is invalid ... ok
Name Stmts Exec Cover Missing
---------------------------------------------------
osol_install 1 1 100%
osol_install.logger 151 126 83% 73-74, 83, 122, 134-135,
171-179, 187-190, 229, 264, 268, 282-287
---------------------------------------------------
TOTAL 152 127 83%
----------------------------------------------------------------------
Ran 20 tests in 2.300s
OK
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
