Thank you Karen. I agree with all your comments below. I will have an
updated webrev out shortly.
- Geoffrey
On 05/31/12 17:20, Karen Tung wrote:
Hi Geoffrey,
A couple nits on the changes:
- Can you add a comment on the changes you made
indicating what that code is doing.
- You have 60 hard coded everywhere. I think it
will be useful to define it as a constant, so, if we feel
that reporting progress every 60 seconds is too much/little,
we can easily tune the value.
Can you update the bug to say what the fix is?
In the Evaluation section, you mentioned that you will add
logging "periodically" to inform the progress. In the comment
section, you show the sample output of reporting progress
every minute. From reviewing the code, I can see that your update
is to report progress every minute. Can you clearly spell that out
in the bug report?
Thanks,
--Karen
On 05/31/12 13:03, Geoffrey Hart wrote:
Afternoon all!
Could I please have a code review for:
BUG: 7171973 IPS transfer code should show more logging during
install of packages
CODE REVIEW:
https://cr.opensolaris.org/action/browse/caiman/ghart/IPS_Install_Progress/
DESCRIPTION: Adding progress logging during the install of IPS
packages so during the 10+ minutes of install time there are now log
entries to show the install is proceeding and not hung.
OLD LOG ENTRY:
2012-05-29 16:19:30,799
InstallationLogger.generated-transfer-1653-1 DEBUG Install Phase
... Started.
2012-05-29 16:31:40,035
InstallationLogger.generated-transfer-1653-1 DEBUG Install Phase
... Done.
NEW LOG ENTRY:
2012-05-29 16:19:30,799
InstallationLogger.generated-transfer-1653-1 DEBUG Install Phase
... Started.
2012-05-29 16:20:30,803
InstallationLogger.generated-transfer-1653-1 DEBUG 24% complete.
2012-05-29 16:21:30,804
InstallationLogger.generated-transfer-1653-1 DEBUG 32% complete.
2012-05-29 16:22:32,195
InstallationLogger.generated-transfer-1653-1 DEBUG 40% complete.
2012-05-29 16:23:32,239
InstallationLogger.generated-transfer-1653-1 DEBUG 47% complete.
2012-05-29 16:24:32,241
InstallationLogger.generated-transfer-1653-1 DEBUG 55% complete.
2012-05-29 16:25:32,264
InstallationLogger.generated-transfer-1653-1 DEBUG 61% complete.
2012-05-29 16:26:32,347
InstallationLogger.generated-transfer-1653-1 DEBUG 64% complete.
2012-05-29 16:27:32,365
InstallationLogger.generated-transfer-1653-1 DEBUG 72% complete.
2012-05-29 16:28:34,229
InstallationLogger.generated-transfer-1653-1 DEBUG 75% complete.
2012-05-29 16:29:34,291
InstallationLogger.generated-transfer-1653-1 DEBUG 81% complete.
2012-05-29 16:30:36,185
InstallationLogger.generated-transfer-1653-1 DEBUG 85% complete.
2012-05-29 16:31:36,189
InstallationLogger.generated-transfer-1653-1 DEBUG 98% complete.
2012-05-29 16:31:40,035
InstallationLogger.generated-transfer-1653-1 DEBUG Install Phase
... Done.
TESTING:
- Confirmed that new logging works and appears in this new format.
- Verified that the install still succeeds and the OS boots correctly
after install
Thanks,
Geoffrey
_______________________________________________
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
