Code review comment for lp:~spiv/bzr/per-file-merge-hook-491711

>>>>> "Andrew" == Andrew Bennetts <email address hidden> writes:

    Andrew> Vincent Ladeuil wrote:
    Andrew> [...]
    >> 41 +def cachedproperty(getter_func):
    >> 42 + """Like property(getter_func), but caches the value.
    >> 43 +
    >> 44 + Note: there's no way to clear or reset the cache.
    >> 45 + """
    >> 46 + memo = []
    >>
    >>
    >> AIUI, this is gc'ed when no more references exist on the cached
    >> value. It's not obvious and may be worth documenting.

    Andrew> Erk, this is totally bogus.

Really ? How so ?

<snip/>

    >> The hook scope is a bit unclear regarding the tt updates
    >> (spotted when reading revno 4887). Why don't you let the
    >> hooks do them (in merge.merge_contents()) ? Shouldn't
    >> that be at least documented in the hook docstring ?

    Andrew> I'm not sure what you mean here? You mean let the
    Andrew> hook do the self.tt.version_file and
    Andrew> self.tt.delete_contents calls that happen at the end
    Andrew> of merge_contents?

Yes or let the caller do it but document it anyway.

    Andrew> I guess I'm still not certain exactly how much the
    Andrew> hook should be allowed to do... the more plugins
    Andrew> people try to write for it, the more we'll know :)

Ok, no problem, I'm not sure either. It's just that it seems odd
to *forbid* the hook to do some stuff a bit different if
needed. But there is no point forcing people to do it
unconditionally either. Time will tell.

I'm happy enough with what is possible today anyway.