[Asterisk-code-review] cdr/cdr odbc.c: Added to record new columns add on CDR 1.8 A... (asterisk[master])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: cdr/cdr_odbc.c: Added to record new columns add on CDR 1.8 Asterisk Version
......................................................................


Patch Set 3: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/144/3/cdr/cdr_odbc.c
File cdr/cdr_odbc.c:

Line 85: "%s" ,",
Fix the spacing here. Commas should have spaces after them, not before them:

"%s", ",peeraccount,linkedid,sequence"


Line 86: "%s" ,",?,?,?"
Fix spacing issues here as well.


Line 93: ,table
Space between ',' and 'table'.


Line 278: 		if (((tmp = ast_variable_retrieve(cfg, "global", "newcdrcolumns"))) && ast_true(tmp)) {
While this was documented in 11/13 in the UPGRADE file (which is correct), it needs to be documented in CHANGES for 14.0.0.

Think of it this way:
1) If you are running 11/13 and upgrade to the next point release, you need to know what features affect your upgrade midstream.
2) However, when Asterisk 14 is released, users moving to that version may or may not be running 11/13, in which case the ability to support said columns is a 'new feature'.

I'd put the following in CHANGES:

(after applications, but before channel drivers):

CDRs
------------------

cdr_odbc
------------------
 * Added a new configuration option, "newcdrcolumns", which enables use of the
   post-1.8 CDR columns 'peeraccount', 'linkedid', and 'sequence'.


https://gerrit.asterisk.org/#/c/144/3/configs/samples/cdr_odbc.conf.sample
File configs/samples/cdr_odbc.conf.sample:

Line 12: Enable log new cdr columns (peeraccount, linkedid, sequence)
I'd rephrase this to:

Enable logging of post-1.8 CDR columns (peeraccount, linkedid, sequence)


-- 
To view, visit https://gerrit.asterisk.org/144
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe0c7540a88305c6012786f438a0813ad8b19127
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Rodrigo Ramirez Norambuena <decipher.hk at gmail.com>
Gerrit-HasComments: Yes