[Asterisk-code-review] app confbridge: Add talking indicator for ConfBridgeList AMI... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/9060 )

Change subject: app_confbridge: Add talking indicator for ConfBridgeList AMI response
......................................................................


Patch Set 3:

(1 comment)

The patch is fine as far functionality is concerned.

The sticking point is the policy that new features need testsuite tests when added to released branches.  The patch falls into a gray area because it now sends out current state information on an existing AMI event.  What is the needed test to check?  The ability to determine talking status?  The presence of the Talking AMI header in the event?

Also since this is changing an AMI event, the AMI version needs to be bumped (but NOT by this patch) appropriately when the next Asterisk release is made.

A note in CHANGES in the appropriate place is needed to document that the AMI event now has the new header available.  Where the note needs to go depends if the patch is going into v13/v15 or just in master.

https://gerrit.asterisk.org/#/c/9060/3/apps/app_confbridge.c
File apps/app_confbridge.c:

https://gerrit.asterisk.org/#/c/9060/3/apps/app_confbridge.c@3540
PS3, Line 3540: 		"Talking: %s\r\n"
> This is shown even when talk_detection_events=no. Should this only be shown
No.  This should always be present just like Muted is always present.



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I19b5284f34966c3fda94f5b99a7e40e6b89767c6
Gerrit-Change-Number: 9060
Gerrit-PatchSet: 3
Gerrit-Owner: William McCall <william.mccall at gmail.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: William McCall <william.mccall at gmail.com>
Gerrit-Comment-Date: Thu, 31 May 2018 13:01:52 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180531/cddfc05c/attachment-0001.html>