Code review comment for lp:~vila/bzr/690563-better-env-isolation

Revision history for this message
Martin Packman (gz) wrote :

Agreed that _captureVar is dodgy and we want a public interface that does the right thing instead. Will need to fiddle with these environment things further to fix bug 321320, as doctests can't use TestCase methods to provide isolation.

+ if name in self._old_env:
+ # We already protect this variable so we should not record its
+ # initial value
+ osutils.set_or_unset_env(name, value)
+ else:
+ self._old_env[name] = osutils.set_or_unset_env(name, value)

I'd probably spell this in a positive way instead:

    old_value = osutils.set_or_unset_env(name, value)
    # Save the original value provided it's not already been recorded
    if name not in self._old_env:
        self._old_env[name] = old_value

review: Approve

« Back to merge proposal