[asterisk-dev] [Code Review] New manager actions for xmldoc

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/627/#review1923
-----------------------------------------------------------



/trunk/include/asterisk/manager.h
<https://reviewboard.asterisk.org/r/627/#comment4150>

    New additions to the public API should have an AST_ prefix



/trunk/include/asterisk/manager.h
<https://reviewboard.asterisk.org/r/627/#comment4151>

    I'm not a huge fan of this part of the patch.
    
    It breaks API and ABI.  While it is trunk, we should always avoid both types of change when possible.
    
    I don't understand the purpose of registering a manager event.  It seems to be overloading the register API call for actions, which is for registering callbacks.  If a register API call events really is needed, then I'd rather see it as a separate API call, instead of overloading this one and causing the API disruption in the tree.
    
    Instead of requiring registration of each event, why not just open the XML doc and traverse it for ami_events?


- Russell


On 2010-04-27 09:08:53, pabelanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/627/
> -----------------------------------------------------------
> 
> (Updated 2010-04-27 09:08:53)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds 2 new elements for xmldoc: <ami_command> (replacing <manager>) and <ami_event>.
> 
> I have also modified the structure for manager_action to include a type field, creating MANAGER_COMMAND and MANAGER_EVENT.  The major difference with MANAGER_COMMAND and MANAGER_EVENT is events will pass NULL for the function parameter.
> 
> The majority of this patch is converting <manager> to <ami_command>, this was only done for a cosmetic reason.  If the people consider this a big change, I can revert to using <manager>.
> 
> *NOTE* This patch is currently missing updates for in-line documentation.
> 
> 
> This addresses bug 17173.
>     https://issues.asterisk.org/view.php?id=17173
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_meetme.c 258973 
>   /trunk/apps/app_mixmonitor.c 258973 
>   /trunk/apps/app_queue.c 258973 
>   /trunk/apps/app_senddtmf.c 258973 
>   /trunk/apps/app_voicemail.c 258973 
>   /trunk/channels/chan_agent.c 258973 
>   /trunk/channels/chan_dahdi.c 258973 
>   /trunk/channels/chan_iax2.c 258973 
>   /trunk/channels/chan_sip.c 258973 
>   /trunk/channels/chan_skinny.c 258973 
>   /trunk/doc/appdocsxml.dtd 258973 
>   /trunk/include/asterisk/manager.h 258973 
>   /trunk/main/data.c 258973 
>   /trunk/main/db.c 258973 
>   /trunk/main/features.c 258973 
>   /trunk/main/manager.c 258973 
>   /trunk/main/pbx.c 258973 
>   /trunk/main/xmldoc.c 258973 
>   /trunk/res/res_agi.c 258973 
>   /trunk/res/res_jabber.c 258973 
>   /trunk/res/res_monitor.c 258973 
>   /trunk/res/res_mutestream.c 258973 
> 
> Diff: https://reviewboard.asterisk.org/r/627/diff
> 
> 
> Testing
> -------
> 
> Local developers and test box.
> 
> 
> Thanks,
> 
> pabelanger
> 
>