[asterisk-dev] [Code Review] rework cdr_csv parsing (2nd attempt)

[Tilghman Lesher]

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


I don't know that it's a reasonable assumption to make that if a config file is missing, that we should activate the module anyway, especially with a module which logs to disk.  Anything of the sort really should need an explicit declaration that the administrator REALLY wants data going to disk.  Compare what the internal logger does if logger.conf is missing:  it logs nothing to disk, because it has received no instruction to do so.  The module cdr_csv.so should behave similarly:  log nothing to disk unless the configuration file explicitly instructs it to do so.

- Tilghman


On Sept. 23, 2011, 1:16 p.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1456/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2011, 1:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a follow up to a previous patch for cdr_csv.  Now that a few other modules have been updated I took a look back at this one.  We now check the ast_config_load() flags like other modules. But the major change, we will now load the module with default settings, even if a configuration files is missing.  Again, this makes the module work more like other modules.
> 
> This also include some coding guidelines fixes.
> 
> 
> Diffs
> -----
> 
>   trunk/cdr/cdr_csv.c 337894 
> 
> Diff: https://reviewboard.asterisk.org/r/1456/diff
> 
> 
> Testing
> -------
> 
> tested on local development box
> 
> 
> Thanks,
> 
> Paul
> 
>

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