[asterisk-dev] [Code Review]: Add MixMonitor and StopMixMonitor AMI actions

[David Vossel]

> On Dec. 2, 2011, 1:03 p.m., jrose wrote:
> > I think this is ready as-is. It's trunk only since these are new AMI actions, so go ahead and document it in CHANGES as well.
> 
> jrose wrote:
>     By the way, if there isn't any feedback on this within a week or so, I'll go ahead and commit it myself.
> 
> Olle E Johansson wrote:
>     We really have tried to avoid separate start/stop actions for a long time. I really would like this to follow the syntax we have followed for some releases.
> 
> wdoekes wrote:
>     While I do prefer to not pollute the Action namespace -- I too like a separate Operation parameter -- I find no evidence of this being standard:
>     
>     The only "Operation" parameter I can find is:
>     - Filter -> Operation (Add)
>     
>     Having separate actions is far more common:
>     - Confbridge*
>     - DAHDIDND*
>     - Meetme*
>     - Qeueue*
>     - *Monitor
> 
> Olle E Johansson wrote:
>     Most of these are old, except ConfBridge - I must have missed that review. We haven't used "Operation" as much as "status: on/off" and others. There are several examples of that. We had an agreement a long time ago that this was the recommended architecture for new manager actions.
> 
> jrose wrote:
>     Filter only seems to have one operation as well.  If this is meant to be the current standard, I can't really see any examples of it having been followed.
>     
>     Since this is only going into trunk, I don't really see any problem with committing the patch though. There is still plenty of time before Asterisk11 even starts being talked about for release purposes, so naming issues like this are malleable for the time being, and the patch to change it to work like you are describing would be fairly trivial.

The StopMixMonitor and MixMonitor AMI actions are consistent with the Dialplan Application's they mimic.   If StopMixMonitor ever gets any arguments passed in, then we have a giant mess with the AMI actions if we use the same action for both starting and stopping.

With that said.  I am backing the use of the StopMixMonitor and MixMonitor AMI actions.


- David


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


On April 21, 2011, 9:29 a.m., telecos82 wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1193/
> -----------------------------------------------------------
> 
> (Updated April 21, 2011, 9:29 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Add MixMonitor and StopMixMonitor AMI actions to allow starting and stopping MixMonitor from AMI.
> 
> 
> This addresses bug 19155.
>     https://issues.asterisk.org/jira/browse/19155
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/branches/1.8/apps/app_mixmonitor.c 314406 
> 
> Diff: https://reviewboard.asterisk.org/r/1193/diff
> 
> 
> Testing
> -------
> 
> Tested with SIP, IAX and DAHDI channels.
> 
> 
> Thanks,
> 
> telecos82
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111205/c3a77fb0/attachment.htm>