Questions regarding hooks

[Didly Bom]

On Fri, Jun 18, 2010 at 11:25 AM, Martin Geisler <mg at aragost.com> wrote:

> On Thu, Jun 17, 2010 at 15:32, Didly Bom <didlybom at gmail.com> wrote:
>
> > The reason I ask about this is that I want to improve the notify
> extension
> > so that you do not need to manually configure its hooks ( changegroup or
> > incoming), which I find complicates the setup of that extension
> needlessly.
>
> In the particular case of notify, I think adding the hook is the least
> part of the configuration. As I remember, there is already a fair
> amount of configuration going on with that extension, so this is just
> one step out of many.


It is true that there is a bit of configuration that you may add to the
notify extension setup. Nevertheless, most of that configuration is shared
with other extensions that must use email (such as the patchbomb extension).
That configuration is easily accessible from GUI tools such as TortoiseHg.
However, those GUI tools do not give easy access to setting up hooks, which
I think may be considered "advanced" mercurial configuration.

That is why I find that having to manually setup a hook makes it harder than
it should. In addition you must remember that the hook must point to
"python:hgext.notify.hook" (funnily enough, you made that exact comment
later on your email : "# I don't remember the exact name of the hook" :-) )

So while this is only a small part of the notify extension configuration I
find that it is the hardest (and hardest to remember). Adding a default
automatic changegroup hook will solve that issue.

> The only doubt that I have is if I need to take into account that any
> > current user of the notify extension will already have set up a
> changegroup
> > or incoming hook manually (hence my question about setting the same hook
> > several times).
>
> Yes, that is a problem... you cannot predict the name used by the user
> -- he may have configured it as
>
>  [hooks]
>  changegroup.send-to-me = notify:hook # I don't remember the exact
> name of the hook
>
> and you would have to recognize this somehow.


I'd like to use the following function to check whether the user manually
configured a notify hook:

#--- Begin of python code ---
def is_notify_hook_manually_configured(ui):
    '''Return True if a hook to the "notify.hook" function has been
configured'''
    b_notify_hook_is_setup = False
    v_hook_list = ui.configitems("hooks")
    for v_hook_item in v_hook_list:
        if v_hook_item[1].find(".notify.hook") != -1:
            b_notify_hook_is_setup = True
            break

    return b_notify_hook_is_setup
#--- End of python code ---

That is, rather than checking if the user configured the changegroup or
incomming hooks I check if there is _any_ hook that points to the
"notify.hook" function. As far as I can tell the user must point to
"notify.hook" in order to use the notify extension. Therefore I think this
code should catch all possible manual configurations of the notify
extension.

I do not want to also check that the hook is for the changegroup or the
incoming events, since perhaps the user found a creative use of the notify
extension that involves other types of hooks.

Do you think this is a good solution?

> The alternative is not to worry about that since it would be an easy thing
> > to change but it does not feel right to me.
>
> Yeah, such incompatible changes are not good. We want people to be
> able to upgrade Mercurial with the least amount of hassle and so we
> should not require people to change already working setups.
>

That is exactly what I thought. So my idea is to use
the is_notify_hook_manually_configured() function described above and only
configure a default changegroup hook when the function returns False.


> New extensions can add hooks using the mechanisms you've discovered --
> the eol extension is an example of this.
>
> > Also, I don't know how to create a test for this change. I can test it on
> my
> > machine of course, but I cannot verify that the notification email is
> > changed. Do you have any ideas of how to proceed regarding testing of
> this
> > change?
>
> There is probably already a test case for notify -- look for
> tests/test-notify or something like that.
>

OK, I will give that a look.

Thanks,

Angel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20100618/616036ce/attachment.htm>