Code review comment for lp:~jr/bzr/707274-commit-message-hook

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/4/2011 4:34 PM, Jelmer Vernooij wrote:
> Review: Approve code
> I'm surprised by how small this is, well done.
>
> A minor nitpick: You need an extra empty line after set_commit_message.
>
> Approving, though it would indeed be useful if one of the qbzr folks could also have a look.

In talking with Jonathan about this (voice-chat no less), it doesn't fit
the qbzr use case, but they really need something custom anyway.
Specifically, they already select files before committing (so they run
'status' and then let the user use radio boxes, etc.)

They want to get the templated commit message before actually running
commit (which the current api, nor the new api support). And then they
can set the commit message with just the equivalent of '-m' or '-F'. So
they don't need a hook like this (bzr-builddeb does). What they need is
a hook that lets template messages be generated without actually having
processed a commit at all.

Note that there are lots of fuzzy bits to that. Because if you haven't
actually processed a commit, what if the code actually wants to insert
the names of the new files, or a per-file comment, or ... We can evolve
to that. Eg as a first pass, having a templated commit message that does
*not* take the list of files, or having it take the list, but qbzr needs
to re-query whenever a user changes the selection list.

All that to say, this handles what bzr-builddeb needs, and qbzr needs to
design something that is going to work for them, because it is very
different from the existing stuff. So we shouldn't block on this.

My only concern is if we would rather just have a flag on the existing
hook, that says "if the user uses this template message without changing
it, just commit the text, don't prompt them if they really want it.".
(Extend the existing hook, rather than creating a new one.)

However, I don't think the current hook is easy to extend, so I'm good
with this.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3s4wUACgkQJdeBCYSNAANd7QCg1Zjob46KFud1AJRxKDqmparL
ONUAniPGsNswxVwtUsAt0l6xAPCnk+jt
=Rx7Z
-----END PGP SIGNATURE-----

« Back to merge proposal