Keith Mitchell wrote: > I'd suggest setting BASEDIR to "" (empty string) when self.LIVECD and > self.AUTO_INSTALL are False. Then you can just always set SOME_VAR = > BASEDIR + SOME_OTHER_VAR, without having to check each time. > > This may require adding a second variable, if BASEDIR has other uses. >
Since I've removed this check from the latest version so this doesn't really make since for this fix. However it definitely does make sense for the fix to 7880 so based on that and the fact that it may get confusing just adding this to 7880 I've combined the fixes for 7880, 11436 and 364 and created a combined webrev. http://cr.opensolaris.org/~evanl/364,7880/ This webrev includes all the comments from both code reviews for bugs 7880, 11436 and 364. This also includes changes for another comment that Keith made about the exception handing for set_homedir_map in ict.py. Thanks, -evan > - Keith > > Evan Layton wrote: >> Fair enough I can easily remove this check for AUTOHOME. >> >> For the fix to 7880/11436 I know of no other way to differentiate this >> and it is needed for the use of add_splash_to_menu in libbe. >> >> -evan >> >> Sarah Jelinek wrote: >>> I don't really like adding this type of check, unless its absolutely >>> required because every time we add a new installer app we have to >>> modify this code. >>> >>> Is there another way we can tell if differentiation is required? >>> >>> thanks, >>> sarah >>> **** >>> >>> Evan Layton wrote: >>>> Dave Miner wrote: >>>>> Jack Schwartz wrote: >>>>>> Hi Sarah and Evan. >>>>>> >>>>>> On 10/13/09 06:44, Sarah Jelinek wrote: >>>>>>> Hi Evan, >>>>>>> >>>>>>> Looks ok. I do have a question: >>>>>>> >>>>>>> Why do we have this check: >>>>>>>> if ((self.LIVECD_INSTALL) or (self.AUTO_INSTALL)): >>>>>>>> 365 self.AUTOHOME = BASEDIR + AUTOHOME >>>>>>>> 366 else: >>>>>>>> 367 self.AUTOHOME = AUTOHOME >>>>>>>> 368 >>>>>>> >>>>>>> In ict.py? >>>>>>> >>>>>>> And, what happens when we add the text installer product? Don't >>>>>>> we have to modify this to include a check for that? >>>>>> Text-install image does introduce a third type here. I'll address >>>>>> this as part of the text-installer modification (part of the >>>>>> text-mode-menu work I've got for Driver Update). >>>>>> >>>>> >>>>> I think the real question is why this is attempting to >>>>> differentiate at all. In what situations would this ever be called >>>>> that BASEDIR shouldn't be prepended? >>>> >>>> I can't think of one currently however it is conceivable that we may >>>> want to run this outside of the installer similar to >>>> add_splash_to_menu. If that is the case then we would not want to >>>> prepend BASEDIR. It was for this possibility that I added this check. >>>> >>>> -evan >>>> >>>>> >>>>> Dave >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
