Hi Jan, Here are my comments:
auto_install.c: lines 920-933: Why was this removed? perform_slim_install.c: line 240: be_attrs need to be freed here as well. perform_slim_install.c: lines 662, 667, 675: Should these say "pkg(5) packages" instead? live-fs-root: line 396: s/server/install server Alok On Tue, 27 Oct 2009, Jan Damborsky wrote: > Hello, > > in the light of the new policy announced yesterday (holding on pushes > of Python files not related to Python 2.6 transition), let me clarify > how I can see it affects the effort of fixing bucket of AI error > reporting bugs. > > [1] code review > --------------- > Since all Python files will be made PEP8 compliant as part of Python2.6 > transition, I have removed all PEP8 related modification from Python > files - this actually makes the Python changes straightforward and easier > to review - new simplified PEP8-less webrev is available here: > > http://cr.opensolaris.org/~dambi/ai-err-report-no-pep8/ > > No new diffs to non-Python changes have been introduced. > > I don't anticipate it should affect the code review schedule > (current deadline for providing code review comments is set to COB > Wednesday October 28th, 2009), but feel free to let me know if it > should be extended, since I can see that people's availability is > limited due to the Python 2.6 transition effort. > > [2] push > -------- > According to the current policy, changes will be pushed after 2.6 stuff > is integrated (after merged and retested). > > Thank you, > Jan > > > > Jan Damborsky wrote: >> Hi, >> >> Could I please ask for reviewing the fix for bucket of AI bugs >> related to error reporting ? >> >> Please provide your code review comments by COB Wednesday >> October 28th, 2009. If you need more time, please let me know. >> >> Thank you very much, >> Jan >> >> * Background >> ============ >> >> As far as AI error handling and reporting is concerned, there is >> a plan to hook AI into new error handling service which is to be based >> on work Evan initiated. >> As it was identified that it will take some time to implement the >> final solution (there are still things to be sorted out and dependencies >> to be addressed - e.g. CUD effort, progress reporting or new >> logging service), we decided to address existing bugs filed against AI >> error reporting - those which are annoying or might be source of >> confusion, but which doesn't involve too much effort to fix, since it is >> likely that some portion of the changes will go away once AI switches to >> new error and logging service. >> >> * Following bugs are addressed >> ============================== >> >> 6651 autoinstall needs more useful error messages from Orchestrator >> module >> 7433 autoinstall should include output from pkg(5) >> 8127 Complete /tmp/install_log not copied to >> /var/sadm/system/logs/install_log >> 8639 be_init() failed with error code 4044 when username is "" >> in SC manifest >> 10040 Error reporting when unable to download >> solaris.zlib/solarismisc.zlib is unobvious >> 10688 AI should validate repository reachability >> 11421 ddm_drive_is_cdrom(): ioctl(DKIOCREMOVABLE) >> failed in /tmp/install_log >> 11640 decrypt liborchestrator IPS debug messages >> >> * Webrev >> ======== >> >> http://cr.opensolaris.org/~dambi/ai-err-report/ >> >> * list of files modified - associated with bugs >> 6651 >> - auto_install.c >> - installation-screen.c >> - om_misc.c >> - orchestrator_api.h >> - perform_slim_install.c >> >> 7433 >> - transfer_mod.py >> - install_utils.py >> - libtransfer.c >> >> 8127 >> - auto_install.c >> - ict.c >> - orchestrator_api.h >> - perform_slim_install.c >> >> 8639 >> - auto_install.c >> - auto_parse.c >> >> 10040 >> - live-fs-root >> >> 10688 - addressed by bug 7433 >> >> 11421 >> - td_dd.c >> >> 11640 >> - perform_slim_install.c >> >> * PEP8 effort >> ============= >> >> Following Python files were made PEP8 compliant and 'pylinted' >> (they are not pylint clean, but has ratio >7/10 with the exception >> mentioned below): >> >> - install_utils.py (no Errors, no Fatals) - rated at 8.71/10 >> >> Following green Cdiff webrev sections belong to bug fixes, everything >> else is PEP8 related: >> --- 404,414 ---- >> --- 424,441 ---- >> --- 451,539 ---- >> >> >> - install-finish (no Errors, no Fatals) - rated at 8.85/10 >> >> Following green Cdiff webrev sections belong to bug fixes, everything >> else is PEP8 related: >> --- 212,238 ---- >> >> >> - transfer_mod.py (No Fatals)- rated at 0.33/10 >> >> The problem of this file is following section: >> ... >> execfile('/usr/lib/python2.4/vendor-packages/osol_install/transfer_defs.py') >> ... >> I have verified that all errors generated by pylint(1) complain about >> undefined symbols coming from that file. >> I was thinking about addressing this problem as Jean suggested (import >> explicitly all symbols used). It worked in case of 'install-utils.py', >> but it turned out that transfer_mod.py uses values which are generated >> on-the-fly, so it didn't help. I have found bug 5559 which will change >> the format of transfer_defs.py anyway, so I decided not to address >> that problem by this fix. >> >> Following green Cdiff webrev sections belong to bug fixes, everything >> else is PEP8 related: >> --- 21,67 ---- >> --- 318,327 ---- >> --- 830,852 ---- >> --- 889,904 ---- >> --- 928,946 ---- >> --- 976,999 ---- >> --- 1011,1025 ---- >> --- 1044,1059 ---- >> --- 1100,1130 ---- >> --- 1146,1160 ---- >> >> >> * Testing >> Please see following file for test used procedures and test results: >> >> http://cr.opensolaris.org/~dambi/ai_error_reporting-test/test-procedures.txt >> >> Also Jeffrey Huang is involved in testing process. >> >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
