Thanks Evan. I'll make these changes.
ginnie
On 08/12/10 04:38 PM, Evan Layton wrote:
Hi Ginnie,
It looks like you've addressed most of the comments already received
and things look pretty good. I did find a couple of things but mostly
minor things.
in lib/install_logging/logger.c
line 1695 - the fprintf should be removed (probably just a debugging
oversight).
install_logging/test/test_add_progress_handler.c
After line 58 it should probably also be checking that handler_args !=
NULL.
The same is true for the rest for the test code where you're calling
nvlist_alloc().
install_logging/test/test_set_logger_class.c
Why all the blank lines from lines 57 to 66?
install_logging/test/test_setup.h
line 32 is commented out should it be removed?
install_logging/test/test_transfer_destonly.c
Some of the indenting appears that it may be off a bit make sure to
run cstyle on all of these.
install_logging/test/test_transfer_srclog.c
should transfer_args be freed after line 90?
Also should transfer_args be freed after line 100 or before line 107?
Other than that the C code looks fine to me. :)
-evan
On 8/9/10 5:11 PM, 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
