Hi Glenn, Thanks for your review of the text installer code. Sorry it's taken a while to respond back. Responses below ..
On Thu, 17 Dec 2009, Glenn Lagasse wrote: > Hi Keith, > > * Keith Mitchell (Keith.Mitchell at Sun.COM) wrote: >> Hi Glenn and all other reviewers, >> >> I have posted an updated webrev at: >> http://cr.opensolaris.org/~kemitche/text_v2/ > > I reviewed everything under the prefix of 'cmd/distro_const'. If I > don't call out a file by name then I had no comments for it. > > post_boot_archive_pkg_image_mod: > > line 31: This doesn't seem Auto-install specific anymore ;-) Changed. > plat_setup.py: > > line 26: s/ai_plat_setup/plat_setup/ Changed. > line 38,39: Are these necessary given it appears we don't run this for > x86 images? Deleted. > tm_gen_cd_content: > > line 29: s/tmcd_gen_cd_content/tm_gen_cd_content/ > > lines 32,39: Could use some cleanup to not reference the liveCD since this > is the text installer cd, not the slim liveCD > line 85: do we need to keep the name .livecd-cdrom-content, it's > somewhat misleading imho > line 91: another reference to 'live cd' There isn't a difference between this script and slimcd_gen_cd_content so I've moved it under 'utils' and nuked the slimcd/text_install copies. > tm_pre_boot_archive_pkg_image_mod: > > line 51: could use a (not used) comment > lines 77,81: TMP_DIR isn't used in the script, so why set it? Changed. > text_mode_sparc.xml > > line 152,157: The comment on 152 doesn't match the setting on 157 > line 187,194: The comment on 187 doesn't match the setting on 194 Changed. > text_mode_x86.xml > > line 166, 171: The comment on 166 doesn't match the setting on 171 > line 201, 208: The comment on 201 doesn't match the setting on 208 > line 839,840: in text_mode_sparc.xml you have just <key_value_pairs/>, > should have the same here for consistency Changed. Thanks for the review. Alok
