Decorating the pull operation to accept an extra argument -- can you help improve an ugly solution?
John Arbash Meinel
john at arbash-meinel.com
Wed May 28 21:30:02 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Yoav Blau wrote:
| We are trying to get "bzr pull" to send a mail. One of the arguments
| will be "originating user" (this is the user that ****requested** the
| pull, not the one who is doing the pull, which is always the build
| manager). Our solution was two-fold:
| · Decorate the pull command to accept an additional argument
| (--originating-user) and put it in a global variable (in our own module)
| · Write a post-pull hook which uses both the "result" object
| (which the hook gets as an argument) and the above global variable to
| create and send a mail
| This solution is obviously hackish L
| It would be ideal if the pull command returned the result of the
| "_to.pull()" operation, not merely sent it to the hooks. Then we would
| do away with the hook, and do everything in the decorator. If there is
| some other, saner, solution, it would be interesting to see it…
| Yoav Blau & Moshe Zadka
I think you are meaning that cmd_pull.run() should return the object from the
pull operation. However Command.run() returns the result code which is meant to
be given to the OS. (cmd_pull always returns None indicating success, or raises
an exception, but that is the Command.run() api.)
In general, if there is more fine-control that someone wants other than just
running a command, then the run() function probably has too much in it, which
should be split into separate helper functions.
I think the hook is a more appropriate place to be doing this sort of thing. I'm
a little curious why this is being hooked up to 'pull' instead of something like
'push'. I suppose this is the maintainer who is pulling changes into mainline?
(Is it always pull, or is it sometimes merge + commit?)
If it is maintaining a mainline, *I* would think you would want merge+commit
when possible, and then you have 'commit --author' to indicate who requested the
commit to mainline.
You could also do it as an environment variable "ORIGINATOR=XXX bzr pull", but
that is about as bad as a --originator=XXX going into a global var.
There isn't a whole lot that cmd_pull.run() does for you given your simplified
requirements. (I assume you don't need it to handle bundles *or* branches, etc).
It wouldn't be a lot of work to just copy the code out and have access to the
result object that way.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar