Code review comment for lp:~jameinel/bzr/2.4-overridAttr-non-existant

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/6/2012 12:06 PM, Martin Packman wrote:
> Review: Approve
>
> Looks fine, but I'm not sure having special logic for the case
> where new is uninited and value is uninited makes sense. That
> doesn't seem like something a test author would do deliberately,
> except perhaps if they were later in the test setting the attribute
> to something and wanted the cleanup at the end, in which case the
> value would leak through in this case.
>

It happens explicitly on Windows when I'm testing that the test case
handles when ENOTSUP doesn't exist. Either I support that case, or I
have to add logic around:
 if getattr(foo, "bar", None) is not None:
    self.overrideAttr(foo, "bar")

In other words, I still need to only remove the attribute if it
exists, and I'd rather have that logic in one place.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Cygwin)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlCYzZ4ACgkQJdeBCYSNAAOZ4gCgx2Z8Lk4Ud0wRXD9TlRb2cx8w
tVQAn28G5xt6eGbwuuzy+CeOrM55Fwtx
=em4F
-----END PGP SIGNATURE-----

« Back to merge proposal