[asterisk-dev] [Code Review] 4571: Add usegmtime option to cel_pgsql

[Matt Jordan]

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



/trunk/CHANGES
<https://reviewboard.asterisk.org/r/4571/#comment25661>

    Two findings here:
    
    1) The review shows a red blob. Extraneous white space should be deleted.
    
    2) The formatting and wording here doesn't match other sections. I would use:
    
    cel_pgsql
    ------------------
    * Added a new option, 'usegmtime', which causes timestamps in CEL events
      to be logged in GMT.



/trunk/cel/cel_pgsql.c
<https://reviewboard.asterisk.org/r/4571/#comment25662>

    There's no reason to initialize this to 0. static variables are allocated in the global data segment, and are always initialized to 0.
    
    Technically, this applies to the 'connected' variable as well, but that's outside the scope of this review :-)



/trunk/cel/cel_pgsql.c
<https://reviewboard.asterisk.org/r/4571/#comment25663>

    You actually don't need the variable. See comment below.



/trunk/cel/cel_pgsql.c
<https://reviewboard.asterisk.org/r/4571/#comment25664>

    You don't need the newusegmtime variable here. Instead, this should simply be written as:
    
    tmp = ast_variable_retrieve(cfg, "global", "usegmtime"));
    if (tmp) {
        usegmtime = ast_true(tmp);
    } else {
        usegmtime = 0;
    }
    
    Note that this assumes that not setting the 'usegmtime' variable should also clear whatever the previous value to its default of False - which should probably be the case, since that is the default value.



/trunk/configs/cel_pgsql.conf.sample
<https://reviewboard.asterisk.org/r/4571/#comment25665>

    Extraneous white space.


- Matt Jordan


On March 30, 2015, 8:19 p.m., Rodrigo Ramirez Norambuena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4571/
> -----------------------------------------------------------
> 
> (Updated March 30, 2015, 8:19 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: https://issues.asterisk.org/jira/browse/ASTERISK-23186
>     https://issues.asterisk.org/jira/browse/https://issues.asterisk.org/jira/browse/ASTERISK-23186
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Feature for added an option that lets you specify that the timestamps going into PostgreSQL from CEL log should be in GMT instead of local time.
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/cel_pgsql.conf.sample 406488 
>   /trunk/cel/cel_pgsql.c 406488 
>   /trunk/CHANGES 406488 
> 
> Diff: https://reviewboard.asterisk.org/r/4571/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rodrigo Ramirez Norambuena
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150401/3ff98875/attachment-0001.html>