Hi Karen. Here are my comments:
DC_tm.py, distro_const.py: -------------------------- I think it would be clearer to the user if you had the "Initializing the IPS package image area" message before the "Setting main repository" and "... authority" messages. It will also be clearer if repo and mirrors were grouped using indentation under authorities. Something like this is what I have in mind: Initializing the IPS package image area: rpool/dc/build_data/pkg_image Setting main authority: opensolaris.org repo: http://pkg.opensolaris.org mirror: pkg.mirror1.os.org mirror: pkg.mirror2.os.org .... mirror: pkg.mirror10.os.org Setting alternate authority: osalt.org repo: http://pkg.osalt.org mirror: ... ... mirror: ... install_utils.py: ----------------- I think the real fix for 3871 is line 431. I'm not sure if 417, changing stderr_log_level to ERROR is correct. Wouldn't that change mean that errors are printed less than stdout (which is set to DEBUG)? My understanding is that errors print more often than output: - have the simple-output file require a higher threshold to print, than the detailed-output file, - have stdout correlate to a threshold which is met for the detailed-output file but not for the simple-output file, - have stderr correlate to a threshold which is met for both detailed- and simple-output files. 435: I don't think you need out_fd and err_fd in the exceptions list (3rd arg to select). Have you tried without this? babel_cd.xml and remaining files: --------------------------------- Just curious: is babel_cd a defacto-standard name? It doesn't seem like a descriptive name to me. Something like all_lang_slim_cd.xml is more descriptive. I assume it is called babel because the slim_install package is replaced by the babel_install package. Hopefully I didn't miss a discussion on this, but.... It may be easier to maintain a single slim_cd.xml file, with a comment inside to replace the slim_install package with babel_install and a suggestion to use lzma compression. That way there will be only one file to modify should changes occur down the line. Also, this package switch-out can be easily documented by Barbara, and since users will be looking at the manifest anyhow, they will see the comment there. It will probably be easier (or less intimidating) for the user to peruse a single file than to figure out the differences of two files. So, my vote is to get rid of babel_cd.xml. But, if you must have these files, the changes to babel_cd.xml and remaining files look OK. Thanks, Jack Karen Tung wrote: > Hi, > > Please do a code review for changes that fixed the following: > > 3707 Distro constructor should output config info at start of build > 3999 Create a manifest for generating the global CD > 3871 stderr output from python finalizer scripts don't show up in DC logfiles > > webrev: > > http://cr.opensolaris.org/~ktung/bugfixes/ > > Thanks, > > --Karen > > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
