Code review comment for lp:~gz/bzr/lru_cache_refactor

> If something's size does change, then this quickly messes up the size
> tracking. I think it is reasonable to eject an item with the size that
> you added it.

This is true, but if objects cached change size that's already making the cache a dangerous rather than useful thing. I guess it's a question of how much we want to defend against misuse.

> bzr-cvsps-import passes in Branch objects, and uses their
> Branch.unlock() as the cleanup callback. Which does use a per-node
> callback (every branch's unlock is different). We can certainly update
> the tests to reflect this.

My initial thought was to add a new base class without the per-node cleanup, move all the bzrlib users to it, and write a small subclass under the current name that adds callbacks. I could still do this, especially if there is code in launchpad or loggerhead that wants it.

I'm unconvinced by the cvsps use case though, caching locked branches doesn't seem a great idea to start with. It would be simple enough to do this with a per-cache callback though, with something like `getattr(value "unlock")()` but... wait, that's not needed, the plugin as its own lru_cache module and doesn't use the one in bzrlib at all.