Hi Drew and Alok -
Here is my code review comments. I reviewed section 1.
thanks,
ginnie
---------------
Makefile.master
---------------
line 86 - ROOTDC_PROFILE value needs to be aligned with the others
--------------
distro_const.py
--------------
line 57 - a heads up for you guys. the transfer_* module names have
changed. transfer_ips will be ips when I putback after the codereview.
line 64 - the comment isn't tabbed over to line up with the line above
nor for the emit comment (line 94). Further down, register_checkpoints,
for example, you have the commented lines lined up. This should be
consistent.
line 219 - RuntimeError (and throughout)...I looked at using this in
transfer, and read on the python site that (highlighted in yellow,
even). So, I wasn't sure that it was appropriate to use it. Could you
tell me why you're using it instead of something else?
line 228 - The + isn't needed for line continuation inside the
parentheses, from my reading of the python style guide.
line 301 - I'm wondering if hard coding the modules is a good idea. As I
noted above, we've change the name transfer modules, and it's possible
the location could change. Would it be worth considering putting them
into something easier to maintain and change.
line 456 - If you want to distinguish in the log what is actually done
in DC, you could add something to the logger name, for example:
InstallationLogger.DC, rather than only using the name provided by the
engine.
line 491 - is it ok if the zpool is zero? In other words, should zpool
actually equal 1.
lines 566, 574, 578 - I haven't had problems with my log output after
removing the backslash. I don't think you need these if it's inside
parentheses. See my comment for line 228.
----------------
profile/Makefile
----------------
34 generic.xml \
35 livecd.xml \
36 text.xml
Line these up with ai.xml
On 10/19/10 16:46, Alok Aggarwal wrote:
This is a code review request for the rewrite of DC
to make use of CUD. The implementation here closely
follows the design document that can be found here:
http://cvs.opensolaris.org/source/xref/caiman/caiman-docs/distro_constructor/dc_cud_design.pdf
Note that this wad does not include the support for
VMC. The support for VMC will be a separate, follow on
project.
Since the changes are fairly substantial we are breaking
up the review into the following sections:
DC app, RBAC, Makefiles and packaging [1]
Manifests and XML serialization/de-serialization [2]
Pre pkg image mod, boot archive configuration and archive archive
checkpoints [3]
All the other remaining checkpoints [4]
Please sign up to review one or more of the sections
and let us know exactly which ones you will be reviewing.
Webrev location is:
http://cr.opensolaris.org/~aalok/cud_dc
Please have your comments in by COB November 5th.
Thanks,
Drew and Alok
[1] usr/src/Makefile.master
usr/src/Targetdirs
usr/src/cmd/Makefile.targ
usr/src/cmd/distro_const/Makefile
usr/src/cmd/distro_const/distro_const.py
usr/src/cmd/distro_const/cli.py
usr/src/cmd/rbac/Makefile
usr/src/cmd/rbac/exec_attr.distribution-constructor
usr/src/cmd/rbac/prof_attr.distribution-constructor
usr/src/cmd/distro_const/checkpoints/Makefile
usr/src/cmd/distro_const/manifest/Makefile
usr/src/cmd/distro_const/profile/Makefile
usr/src/pkg/manifests/install-distribution-constructor.mf
usr/src/cmd/distro_const/checkpoints/defaultfiles/Makefile
usr/src/cmd/distro_const/checkpoints/defaultfiles/coreadm.default
usr/src/cmd/distro_const/checkpoints/defaultfiles/rtc_config.default
[2] usr/src/cmd/distro_const/__init__.py
usr/src/cmd/distro_const/configuration.py
usr/src/cmd/distro_const/distro_spec.py
usr/src/cmd/distro_const/execution_checkpoint.py
usr/src/cmd/distro_const/manifest/boot_archive_contents_sparc.xml
usr/src/cmd/distro_const/manifest/boot_archive_contents_x86.xml
usr/src/cmd/distro_const/manifest/dc_ai_sparc.xml
usr/src/cmd/distro_const/manifest/dc_ai_x86.xml
usr/src/cmd/distro_const/manifest/dc_livecd.xml
usr/src/cmd/distro_const/manifest/dc_text_sparc.xml
usr/src/cmd/distro_const/manifest/dc_text_x86.xml
usr/src/cmd/distro_const/profile/ai.xml
usr/src/cmd/distro_const/profile/generic.xml
usr/src/cmd/distro_const/profile/livecd.xml
usr/src/cmd/distro_const/profile/text.xml
[3] usr/src/cmd/distro_const/checkpoints/pre_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/boot_archive_configure.py
usr/src/cmd/distro_const/checkpoints/boot_archive_archive.py
usr/src/cmd/distro_const/checkpoints/test/test_pre_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/test_boot_archive_configure.py
usr/src/cmd/distro_const/checkpoints/test/test_boot_archive_archive.py
[4] usr/src/cmd/distro_const/checkpoints/ai_publish_pkg.py
usr/src/cmd/distro_const/checkpoints/create_iso.py
usr/src/cmd/distro_const/checkpoints/create_usb.py
usr/src/cmd/distro_const/checkpoints/custom_script.py
usr/src/cmd/distro_const/checkpoints/grub_setup.py
usr/src/cmd/distro_const/checkpoints/pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/test_ai_publish_package.py
usr/src/cmd/distro_const/checkpoints/test/test_create_iso.py
usr/src/cmd/distro_const/checkpoints/test/test_create_usb.py
usr/src/cmd/distro_const/checkpoints/test/test_custom_script.py
usr/src/cmd/distro_const/checkpoints/test/test_grub_setup.py
usr/src/cmd/distro_const/checkpoints/test/test_pkg_img_mod.py
usr/src/cmd/distro_const/checkpoints/test/testlib.py
_______________________________________________
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
