Code review comment for lp:~torkvemada/bzr/commit_hooks

Revision history for this message
Martin Packman (gz) wrote :

In the past, hooks have been given custom objects with just a selection of fields, see for instance bzrlib.status.StatusHookParams - doing the same thing here would make sense, though there are a lot of parameters passed to Commit.commit and we probably don't want to expose them all. Perhaps just revprops, message (though that's complicated, there are already two commit message hooks fighting over it), committer, and maybe the two time ones.

I'd be careful about what the hook is allowed to modify, particularly a test that sets an invalid revision property (see bzrlib.revision.Revision._check_properties) should raise a ValueError rather than breaking the commit in another manner.

« Back to merge proposal