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

>>>>> John Arbash Meinel writes:

<snip/>

    > We don't use weakrefs in the code very much,
    > and where a None can be an easily accidental return from something
    > where we expect to get an object.

Not, it's not easy, it has to be deliberate. The branch object used here
is a *mandatory* parameter of the constructor.

Creating such an object and keeping it while *deleting* the branch is
not only outside of the obvious use case, it's deliberate sabotage and
as I already mentioned, you already get a traceback.

    > So there isn't a lot of prior art, but I *have* seen a fair number
    > of AttributeError bugs filed that are hard to discover.

    > I think using weakref without being careful about its side-effects is
    > poor form.

Which is why I *am* careful. There is *one* usage so far in a test with
a comment. There will be a *single* other one once this is deployed.

    > Even if it "can't happen". Bugs happen.

You still not answer how you want it to be reported. At the point where
it can occur you only have a traceback at best which is already what
will be reported.