Decorating the pull operation to accept an extra argument -- can you help improve an ugly solution?

[John Arbash Meinel]

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

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…
|
| Thanks,
|
| 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.

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

iEYEARECAAYFAkg9wMoACgkQJdeBCYSNAAMBEQCeP09KvNzRY0W9d8Z66uGz44W5
O4IAn20Zb3SXvlvlWxZ2fPlTjkYmKWdA
=8ev4
-----END PGP SIGNATURE-----