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

Revision history for this message
Martin Pool (mbp) wrote :

>> Assuming you've read the comments above, you know we already have cycles involving branches so I'd rather not introduce a new one when I can avoid it.

I've just re-read the preceding comments about cycles.

My first impression here, apparently the same as John, was that this seemed like premature optimization.

It sounds like you have a rule in your head along the lines of "objects that hold references to long-lived parents should always do so through a weakref", or something like that. As spiv said, "this (using weakrefs to avoid cycles) is a new idiom for us and I think it's reasonable to expect we'll make a few mistakes with it."

Could you perhaps post to the list explaining the general rule you think we ought to follow about avoiding cycles? That may save discussing whether it's necessary case-by-case and it will also help other people use it. Also we can discuss whether we prefer that or a .close or .finalize method.

It seems reasonable that the branch config stay in memory as long as the branch is in memory. Is the problem here (per bug 758652) that we suspect Python is just too bad at collecting cyclic structures and we should just always avoid them?

« Back to merge proposal