Code review comment for lp:~vila/bzr/config-lock-remote

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

> ^- If you are using a weak_ref you need to handle when branch_ref()
> returns None. (because the branch has gone away)

Having encountered the issue while implementing the associated tests, I know it's handled: an exception is raised making it obvious that the branch is dead. Since this reveals a bug in the code I don't plan to explicitly test for it (since the gc is involved it's inherently racy).

> I realize this shouldn't generally happen (I assume the branch is
> holding a direct reference to the config object),

Not yet, but that's the plan yes.

> but it is something
> you should at least be prepared for.

See above, I didn't have to think twice to understand what was happening, really.

> review: approve

Thanks, the pre-requisite branches are still pending though.

« Back to merge proposal