[asterisk-dev] [Code Review] 2424: Sorcery Realtime Wizard + Unit Test

[Joshua Colp]

> On April 4, 2013, 7:14 p.m., Mark Michelson wrote:
> > /trunk/main/config.c, lines 2521-2525
> > <https://reviewboard.asterisk.org/r/2424/diff/1/?file=35448#file35448line2521>
> >
> >     Why do you have this special case for the first item in the list?
> >     
> >     Shouldn't the while loop on its own do what you need?

For some cases within the engines the first argument passed in is "special" and treated differently. Instead of making it a separate explicit argument I opted for this to keep impact lower.


> On April 4, 2013, 7:14 p.m., Mark Michelson wrote:
> > /trunk/main/config.c, lines 2527-2542
> > <https://reviewboard.asterisk.org/r/2424/diff/1/?file=35448#file35448line2527>
> >
> >     This while loop constructs a list of ast_variables in the reverse order of what was passed in. Is there any chance for this to cause an issue?

Order doesn't matter for engines.


> On April 4, 2013, 7:14 p.m., Mark Michelson wrote:
> > /trunk/res/res_sorcery_realtime.c, line 41
> > <https://reviewboard.asterisk.org/r/2424/diff/1/?file=35454#file35454line41>
> >
> >     You're going to want to use a different name than this since several of our realtime schemas expect "id" to be the name of one of the realtime fields.

'id' is an implicit object field when an object is defined as being a sorcery object, to protect against any use of the field by users of sorcery I've added a check which will not allow it to be registered.


- Joshua


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


On April 3, 2013, 6:36 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2424/
> -----------------------------------------------------------
> 
> (Updated April 3, 2013, 6:36 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change has three pieces:
> 
> 1. Conversion of realtime to use ast_variable instead of va_arg
> 2. A res_sorcery_realtime module for using all of the realtime config engines with sorcery
> 3. Unit tests for res_sorcery_realtime
> 
> There should be no functional impact as a result of the ast_variable conversion.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/res_config_mysql.c 384670 
>   /trunk/include/asterisk/config.h 384670 
>   /trunk/main/config.c 384670 
>   /trunk/res/res_config_curl.c 384670 
>   /trunk/res/res_config_ldap.c 384670 
>   /trunk/res/res_config_odbc.c 384670 
>   /trunk/res/res_config_pgsql.c 384670 
>   /trunk/res/res_config_sqlite3.c 384670 
>   /trunk/res/res_sorcery_realtime.c PRE-CREATION 
>   /trunk/tests/test_sorcery_realtime.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2424/diff/
> 
> 
> Testing
> -------
> 
> Ran against an actual realtime config engine, postgresql, to confirm that it works as expected. Ran it against unit tests as well to confirm that the wizard works as expected.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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