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

Revision history for this message
Vincent Ladeuil (vila) wrote :

> It will delete an attribute that did not previously exist. There is *no way* to specify that you want to delete an attribute that already exists. (Hint *patch*)

So you're taking the same syntax as 'patch' to mean something completely different ?

And you don't mention it in the docstring, news entry, cover letter ?

The fact that I mis-read your patch and that you fail to explain the difference with patch clearly demonstrate that other people will fall in the same trap.

Not to mention the other trap described by mgz.

A far safer and clearer idiom would be:

overrideAttr(obj, name) # Guarantees isolation
delattr(obj, name) # Testing what happens when name is deleted

« Back to merge proposal