On 2nd thought theses shouldn't take much time to address. Will do.
Joe Joseph J. VLcek wrote: > Hey Sanjay, > > At this late date in development, if it is OK with you, I would like > to handle this by filing a bug and addressing it after feature freeze. > > Joe > > sanjay nadkarni wrote: >> if I run shcomp -n on the ksh93 files I see the following: >>> nadkarni at skinnymoose:/tmp$ shcomp -n create_vm /dev/null >>> create_vm: warning: line 89: `...` obsolete, use $(...) >>> create_vm: warning: line 89: " quote may be missing >>> create_vm: warning: line 90: `...` obsolete, use $(...) >>> create_vm: warning: line 176: -ir invalid typeset option order >>> create_vm: warning: line 223: `...` obsolete, use $(...) >>> create_vm: warning: line 284: `...` obsolete, use $(...) >>> nadkarni at skinnymoose:/tmp$ shcomp -n export_vm /dev/null >>> export_vm: warning: line 158: `...` obsolete, use $(...) >>> export_vm: warning: line 158: `...` obsolete, use $(...) >>> export_vm: warning: line 213: `...` obsolete, use $(...) >>> export_vm: warning: line 229: `...` obsolete, use $(...) >>> export_vm: warning: line 237: `...` obsolete, use $(...) >> Can you please address these. >> >> -Sanjay >> >> >> Glenn Lagasse wrote: >>> Hi Jack! >>> >>> * Jack Schwartz (Jack.A.Schwartz at Sun.COM) wrote: >>> >>>> Hi Glenn and Joe. >>>> >>>> Regarding: >>>> >>>>> <http://cr.opensolaris.org/~glagasse/webrev.diffs> >>>>> >>>> For round 2, all files look good to me, except I still have some >>>> comments on >>>> im_pop.py: >>>> >>>> 573: If (QUIT_ON_PKG_FAILURE == True) and ips_set_auth fails, do >>>> calls to >>>> ips_cleanup_authorities() get made to clean up the UNSET_AUTH_LIST? >>>> Likewise >>>> for ips_cleanup_mirrors() and UNSET_MIRROR_LIST? Are such calls >>>> necessary? >>>> The main just raises an exception and bails. >>>> >>>> Since this is lifted from existing code (DC_tm.py) and you don't feel >>>> comfortable changing it, I'm happy with your offer to file a bug. >>>> >>> >>> Ok. I'll file a bug. >>> >>> >>>> - - - >>>> >>>> The use of PKGFILE between its flush on 523 and its close on 529 >>>> still don't >>>> make sense to me. Close it in one process before using it by >>>> another. The >>>> close will do the flush. What you have will work, but it would be >>>> cleaner to >>>> do this: >>>> >>>> for pkg in PKGS: >>>> PKGFILE.write(...) >>>> PKGFILE.close() >>>> STATUS = ips_pkg_op(PKG_FILE_NAME, ...) >>>> >>>> Likewise for 479-505: >>>> >>>> for pkg in PKGS: >>>> PKGFILE.write(pkg + '\n') >>>> PKGFILE.close() >>>> STATUS = ips_contents_verify(PKG_FILE_NAME, ...) >>>> if STATUS and QUIT_ON_PKG_FAILURE == 'true': >>>> os.unlink(PKG_FILE_NAME) >>>> raise (...) >>>> ... >>>> STATUS = ips_pkg_op(PKG_FILE_NAME, ...) >>>> if STATUS and QUIT_ON_PKG_FAILURE == 'true': >>>> os.unlink(PKG_FILE_NAME) >>>> raise (...) >>>> >>>> os.unlink(PKG_FILE_NAME) >>>> >>> >>> I looked into this, since the transfer module does indeed specifically >>> open the file itself I'm pretty comfortable with this suggestion and so >>> I'll make this change since I agree with what you're saying. >>> >>> >>>> - - - >>>> >>>> Nit: Change QUIT_ON_PKG_FAILURE to be a boolean instead of a string; >>>> then you >>>> can just say >>>> if STATUS and QUIT_ON_PKG_FAILURE: >>>> >>> >>> I'd like to change this, but it will change quite a few places in the >>> code and while seemingly innocuous I already have a couple of >>> regressions in this code that I'm tracking down and I'm leary to make >>> that situation worse. Since this is existing code and I'm not >>> regressing anything I'm going to pass on addressing this other than I >>> can offer to file a low priority bug to clean it up. >>> >>> Thanks Jack! >>> >>> >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > >
