Sue,
image.py
--------
177-180: This seems really heavyweight for getting a new directory.
I'm trying to think if there's any other way we could do this and I'm
coming up empty. I assume that creating the directory based on
timestamp won't work? If not, can you combine 177 and 178?
441: Change to
if fmri is None
446: Change to
if publisher is None
453: Can you try getattr?
if getattr(pkgfrmi, "publisher", None) != new_pub.prefix:
574: Change to
if specified_path is None:
577-580: Can you change to:
return not os.path.exists(os.path.join(aismf.get_imagedir(), svc_name))
597: Change to
if image is None:
651: NIT - align indent
set_service.py
------------
148: no need for ".keys()"
for clientid in clients:
update_service.py
---------------
32: move line to 41
47-51: Why make this a function? Why not just set it on line 61
206-209: Can you change to:
return not bool(setsvc.do_update_basesvc(service, new_service.name))
(I'm not always sure how installadm handles returns from subcommands)
-Drew
On 5/3/12 11:43 AM, Sue Sohn wrote:
Can I get a code review of the changes for:
PSARC/2012/142 installadm update-service
7102885 update a pkg(5) based service
Webrev:
https://cr.opensolaris.org/action/browse/caiman/sohn/7102885/webrev.7102885
Tests:
Command line option validation and action (positive and negative)
Command restricted to pkg(5) based service aliases
Client installs of updated alias
Regression test of set-service -o aliasof and create-service
Ran unit tests (newly added tests pass and no new errors on existing
tests).
In addition, QA has executed all test assertions from the test plan at:
http://onwiki.us.oracle.com/bin/view/SolarisQE/+ISIM+Improvements+Test+Plan
on both sparc and x86 AI servers and has run the installadm test
suites (no regressions).
Supporting requests filed:
o Test Suites:
QE Request: https://sqe-osso.us.oracle.com/sqept/view.php?ticket=29
o Install Guide:
7162893 Install Guide should be updated with new installadm
update-service subcommand, PSARC/2012/142
o installadm(1m) manpage:
7162895 update installadm(1m) man page to include new update-service
subcommand, PSARC/2012/142
Thanks,
Sue
_______________________________________________
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
