On Tue, Jul 1, 2008 at 8:10 PM, James Keenan via RT <[EMAIL PROTECTED]> wrote: > Please review the patch attached. Note the following: > > 1. As mentioned in my last post in this RT, the flow in this step > class's runstep() method is quite convoluted. I tried to improve it, > but this step still has five different points at which it can return. I > have, however, created a distinction between those variables which hold > the user's command-line options and those which hold the current state > of the user's attempt to configure with those options.
Very good. We do want to make sure that command line specification trumps any attempt to probe elsewhere. (User knows best, even if they're wrong.) > 2. I suspect that some of the command-line options haven't been used in > years. Evidence? The fact that one command-line, --icudatadir, is > mentioned in the Configure.pl --help message (held in > lib/Parrot/Configure/Options/Conf.pm) and in README_win32.pod -- but is > not mentioned anywhere in config/auto/icu.pm, or anyplace else for that > matter! The fact that we have lived happily without this option since > r14398 provides the rationale to remove it from the files where it is > mentioned. > > Moreover, I have heard no response to the request in my earlier post for > feedback from people who regularly use the --icushared and --icuheaders > options. I suspect that in practice we could do without those options, > but have taken no action to remove them. I would err on the side of removing them. No point in keeping unused items after the refactor, especially if you're going to end up having to write tests for them. Especially since eventually we're not (as I understand it) going to use ICU. > So, if you have any strong feelings about this patch, please speak up. > Otherwise I will apply it to trunk in 3 days (or 2, if I get impatient). > > Thank you very much. > kid51 -- Will "Coke" Coleda
