Code review comment for lp:~spiv/bzr/debug-flag-relock

Revision history for this message
Andrew Bennetts (spiv) wrote :

John A Meinel wrote:
[...]
> So doing:
>
> foo.lock_read()
> foo.lock_read()
> foo.unlock()
> foo.unlock()
>
> would not trigger it, but
>
> foo.lock_read()
> foo.unlock()
> foo.lock_read()
> foo.unlock()
>
> would.

That's correct.

> I agree that we should probably have a hook, or some sort of mechanism
> for testing this, as it is worthwhile to test. Perhaps you could have a
> global state in "lock" like our "symbol_versioning.warn" functionality?

You mean so that tests can assert “X was locked once”, rather than so that
we can test this debug flag? I think that would be nice, but it's probably
best to let whoever writes the first test along those lines (quite possibly
me!) do that refactoring. i.e. I think it's easy to add this sort of thing
later, and that it is best to add this sort of thing when it will actually
be used so we know we got the API right.

-Andrew.

« Back to merge proposal