[asterisk-dev] [Code Review] GSOC 2010 - Data API providers

[Eliel Sardañons]

> On 2010-06-30 13:55:22, Tilghman Lesher wrote:
> > /trunk/apps/app_voicemail.c, line 10985
> > <https://reviewboard.asterisk.org/r/757/diff/2/?file=11224#file11224line10985>
> >
> >     Probably should be ast_debug()

probably shouldn't be there :D


> On 2010-06-30 13:55:22, Tilghman Lesher wrote:
> > /trunk/apps/app_meetme.c, lines 6742-6743
> > <https://reviewboard.asterisk.org/r/757/diff/2/?file=11222#file11222line6742>
> >
> >     Could we create a type to denote that these values are in seconds or milliseconds, whatever the correct unit is?

Agree


> On 2010-06-30 13:55:22, Tilghman Lesher wrote:
> > /trunk/apps/app_meetme.c, lines 6731-6732
> > <https://reviewboard.asterisk.org/r/757/diff/2/?file=11222#file11222line6731>
> >
> >     Let's think about possibly creating a type that more specifically denotes this as a timestamp, perhaps AST_DATA_TIMESTAMP or AST_DATA_INTEGER_TIMESTAMP.  One possible application of this would be to be able to receive data both as the literal epoch timestamp, as well as a datetime formatted string, and for Asterisk to be able to do the right thing.

Agree


> On 2010-06-30 13:55:22, Tilghman Lesher wrote:
> > /trunk/channels/chan_dahdi.c, lines 17501-17519
> > <https://reviewboard.asterisk.org/r/757/diff/2/?file=11226#file11226line17501>
> >
> >     Seems like these should all be booleans, instead of an encoded string.

Agree


> On 2010-06-30 13:55:22, Tilghman Lesher wrote:
> > /trunk/channels/chan_dahdi.c, lines 17544-17546
> > <https://reviewboard.asterisk.org/r/757/diff/2/?file=11226#file11226line17544>
> >
> >     The ordering of these looks backwards.  According to this, if CRC4 is off, then the line is always "YEL", which should not be the case.

I will review this twice, i just copy the code from what we already have... maybe i don't know how to copy :)


- Eliel


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


On 2010-06-29 22:27:37, Eliel Sardañons wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/757/
> -----------------------------------------------------------
> 
> (Updated 2010-06-29 22:27:37)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> Data API providers already implemented for the summer of code 2010 project
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_meetme.c 273179 
>   /trunk/apps/app_queue.c 273179 
>   /trunk/apps/app_voicemail.c 273179 
>   /trunk/channels/chan_agent.c 273179 
>   /trunk/channels/chan_dahdi.c 273179 
>   /trunk/channels/chan_iax2.c 273179 
>   /trunk/channels/chan_sip.c 273179 
>   /trunk/include/asterisk/cdr.h 273179 
>   /trunk/include/asterisk/channel.h 273179 
>   /trunk/include/asterisk/data.h 273179 
>   /trunk/include/asterisk/indications.h 273179 
>   /trunk/main/cdr.c 273179 
>   /trunk/main/channel.c 273179 
>   /trunk/main/data.c 273179 
>   /trunk/main/indications.c 273179 
>   /trunk/res/res_odbc.c 273179 
> 
> Diff: https://reviewboard.asterisk.org/r/757/diff
> 
> 
> Testing
> -------
> 
> Developer testing.
> 
> 
> Thanks,
> 
> Eliel
> 
>