Code review comment for lp:~ian-clatworthy/bzr/eol-update-bug

On Thu, 2009-09-03 at 00:57 +0000, Ian Clatworthy wrote:
>
> > Commit does not write to the users tree : a post commit hook isn't
> appropriate [for starters, other post commit hooks could interfere/see
> convenience forms themselves etc]. I suggest looking closely for the
> cause/some possible confusion is going on here.
>
> Plugins like keywords do need to update the working tree, if any,
> after
> a commit so that files just committed get keywords values updated. As
> the keywords plugin is currently designed, the only files needing
> potential update are the newly committed ones.
>
> I agree a post commit hook isn't the answer. I've put a separate patch
> up for a finish-commit hook on mutable trees instead.

Oh, that use case wasn't clear - I didn't get what was needed.

Branch.post_commit definitely isn't the right thing as its on branch.

There may be room for MutableTree.post_commit too - the difference being
whether or not <activity> should occur before the basis revision id is
changed.

So, the question to ask is 'should keywords update the files *after* the
basis changes, or *before*. I think the answer is *after*, but you've
spent more time staring at this problem: its up to you.

I do suggest that the hook name try to reflect where in the process its
happening, and if its after commit has done its stuff, its really just a
post commit hook on a different object, so lets use the namespaces and
call it MT.post_commit

-Rob