[asterisk-dev] [Code Review] New "dialplan remove context" and modification of "dialplan add include"

[Mark Michelson]

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


This is mostly good. I suggest taking opticron's suggestion and correcting the minor findings below and this will probably be good to go.


/trunk/pbx/pbx_config.c
<https://reviewboard.asterisk.org/r/2042/#comment13011>

    Coding guidelines require curly braces for single line if statements.



/trunk/pbx/pbx_config.c
<https://reviewboard.asterisk.org/r/2042/#comment13013>

    When submitting code to Asterisk, always ensure that you compile using dev-mode. When you run the configure script, include the command line option '--enable-dev-mode'. This results in more warnings being reported and warnings to be treated as errors.
    
    This line in particular will throw up an error about how mixed declarations and code are forbidden. Declare "con" at the top of the function and set it to ast_context_find() here.



/trunk/pbx/pbx_config.c
<https://reviewboard.asterisk.org/r/2042/#comment13012>

    It is impossible to reach this code. You can just remove it.


- Mark


On July 13, 2012, 4:41 a.m., corruptor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2042/
> -----------------------------------------------------------
> 
> (Updated July 13, 2012, 4:41 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I've noticed that we can remove particular extension from context with dialplan remove extension command but in order to remove all extensions in the context we should delete them on by one. I've created dialplan remove context command which uses ast_context_destroy to destroy the whole context with all extensions. I've created to functions for in pbx_config.c: handle_cli_dialplan_remove_context which actually removes context and complete_dialplan_remove_context which completes input. They are based on other similar functions and pretty trivial but I can be mistaken somewhere.
> 
> I've also modified dialplan add include <context2> into <context1>. I've made it similar dialplan add extension ... command. It creates <context1> if it doesn't exist and I've also modified complete_dialplan_add_include and removed check for existance of <context2> because we can include non-existent context into another one. (I usually include empty (non-existent) contexts in advance). Should we raise warning in this case as it's raised while reading extensions.conf?
> 
> I use those functions with AMI. I think manager commands should be created in addition to those CLI commands.
> 
> 
> Diffs
> -----
> 
>   /trunk/pbx/pbx_config.c 370052 
> 
> Diff: https://reviewboard.asterisk.org/r/2042/diff
> 
> 
> Testing
> -------
> 
> We've used both functions very often for the last couple of months and they seem to work good.
> 
> 
> Thanks,
> 
> corruptor
> 
>

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