On 02/ 3/10 07:19 PM, Karen Tung wrote: > Hi, > > I would like to get at least 2 reviewers to look at changes for > the following 2010.03 stopper. > > 5209 <http://defect.opensolaris.org/bz/show_bug.cgi?id=5209> > Enhancements to the mkrepo script > http://defect.opensolaris.org/bz/show_bug.cgi?id=5209 > > webrev: > > http://cr.opensolaris.org/~ktung/5209/ >
Mostly looks good, comments below. ai_sparc_image.xml 153: "in the order listed" (likewise for 170 in ai_x86_image.xml, 172 in all_lang_slim_cd_x86.xml, 171 in slim_cd_x86.xml) slimcd_live.xml 26: s/Slim// 34, 125, 170: a comment about the timeout change reason would be nice really, we need more comments in all of the install-specific profiles, anyway boot_archive_configure generally, the changes here are stylistically inconsistent in referring to variables; I'm also not sure they're consistent with the preferred coding style for ksh scripts, though I believe we have a general bug about that in the finalizer scripts. It would be nice to not make it worse, though. 238: you've got a potential corner case here in that we require using the build area's DTD for the manipulations here, but the code allows for reference to the build system's profiles. I'd suggest that the DTD reference should be rooted to be consistent with each profile that's applied. 252: line seems unnecessary 263: I would expect that this kind of failure results in a failure of the build. Why should we just continue? generic_live.xml 26: perhaps should say "Base service profile customized..." 69, 83: why not put these under the group starting at 168, since that comment applies to these two? mkrepo I hope we're certain that this will be removed post-EMI integration, otherwise this file really could use a comment noting that it is an adaptation of manifest-import and changes to manifest-import should be evaluated for their applicability here. media-fs-root, 181: A comment on why this is the right point to do this would be a good idea. Smae goes for net-fs-root line 331. Dave
