On Tue, Mar 30, 2010 at 8:02 PM, John Admanski <[email protected]> wrote: > On Tue, Mar 30, 2010 at 2:43 PM, Martin Bligh <[email protected]> wrote: >> >> On Tue, Mar 30, 2010 at 2:30 PM, John Admanski <[email protected]> >> wrote: >> > Isn't setting the variable in os.environ kind of heavyweight...couldn't >> > you >> > set it as part of the utils.system command invocation? That would avoid >> > the >> > mess of having to save off values and adding more cleanup and in general >> > introducing unnecessary state. >> >> Tests are a separate subprocess anyway, so I don't think we even need to >> clean this up. That said, part of utils.system sounds fine too. >> > > True, although I still think it's better to avoid messing with the environ > if you don't really have to. But I'm sure that setting LC_ALL instead of > LANG is the right thing; otherwise people who have LC_ALL or LC_TIME set > will run into the exact same problem that this patch is supposed to address. > And I know there are systems out there with one of those set, because when I > went to try tweaking LANG on my system to try this out it had no effect > because it turned out LC_TIME was defined. :)
Well, as long as the environ is being restored at the end of the test I don't think it's too much of a hassle... anyway, the suggestions are good, I am going to respin the patch. _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
