----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3591/#review12078 -----------------------------------------------------------
Thanks for the feature. This will also require updates to: - CHANGES.txt <-- new features - configs/*pgsql.conf <-- example configs /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22079> Please split this into multiple lines. I know it was like that before, but since you're touching it, you can fix it :) /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22080> No camelCase please. conn_info /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22082> s/pgdbappname/pgappname/ /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22081> Remove these debug statements. - We don't want passwords in the log if it can be avoided. - We certainly don't need the debug message twice. /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22083> No need for this debug statement either. /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22084> - Remove the warning, one shouldn't be required to use this. - For consistency with the surrounding code, add a blank before the ast_free. /trunk/cdr/cdr_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22085> - The connInfo declaration should be at the top of the function. - And the same comments as above apply here. - And since this is now duplicate code, I suggest you move it into a single static pg_connect() function. /trunk/res/res_config_pgsql.c <https://reviewboard.asterisk.org/r/3591/#comment22087> dbappname[0] = '\0'; Try to avoid using strcpy, even when it is safe. And in this case, the above is quicker anyway. - wdoekes On June 6, 2014, 11:23 a.m., doome wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3591/ > ----------------------------------------------------------- > > (Updated June 6, 2014, 11:23 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23737 > https://issues.asterisk.org/jira/browse/ASTERISK-23737 > > > Repository: Asterisk > > > Description > ------- > > See https://issues.asterisk.org/jira/browse/ASTERISK-23737 > > > Diffs > ----- > > /trunk/res/res_config_pgsql.c 415300 > /trunk/cdr/cdr_pgsql.c 415300 > > Diff: https://reviewboard.asterisk.org/r/3591/diff/ > > > Testing > ------- > > Used asterisk with the config paramterer pgdbappname and dbappname in > cdr_pgsql.conf and res_config_pgsql.conf respectively, enabled and disabled. > Worked as expected. > > > Thanks, > > doome > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
