[Asterisk-code-review] apps/app_playback.c: Add 'mix' option to app_playback (asterisk[16])

[Kevin Harwell]

Attention is currently required from: Shloime Rosenblum.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16540 )

Change subject: apps/app_playback.c: Add 'mix' option to app_playback
......................................................................


Patch Set 3: Code-Review-1

(4 comments)

Patchset:

PS3: 
This will need a CHANGES entry

https://wiki.asterisk.org/wiki/display/AST/CHANGES+and+UPGRADE.txt


File apps/app_playback.c:

https://gerrit.asterisk.org/c/asterisk/+/16540/comment/6efa9300_43948c0f 
PS3, Line 55: 				<para>List of options (You can do Playback(filename,skipmix) or Playback(filename,mix|skip) but you can't use a comma as a delimeter).</para>
If a comma separated list was allowed before, then no longer allowing such technically could break current dialplan (even if it seems unlikely).

Why can the options not still be comma separated?


https://gerrit.asterisk.org/c/asterisk/+/16540/comment/4bcf7cf9_22fc405d 
PS3, Line 498: 			else if (option_mix){
add a space between closing ')' and '{'


https://gerrit.asterisk.org/c/asterisk/+/16540/comment/04f3ccfc_e972ae35 
PS3, Line 500: 				if (strcasestr(front, ":") && !strcasestr(front, "://"))
             : 					res = say_full(chan, front, "", ast_channel_language(chan), NULL, -1, -1);
             : 				else
             : 					res = ast_streamfile(chan, front, ast_channel_language(chan));
You could combine this with the "else if" above and then drop the associated else if you wanted. For example:

else if (option_mix && strcase(...) && ....) {

Also please use begin/end brackets for all if/else statements even if they are single line.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16540
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I815816916a308f0fa8f165140dc15772dcbd547a
Gerrit-Change-Number: 16540
Gerrit-PatchSet: 3
Gerrit-Owner: Shloime Rosenblum <shloimerosenblum at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Shloime Rosenblum <shloimerosenblum at gmail.com>
Gerrit-Comment-Date: Wed, 29 Sep 2021 21:13:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210929/65c76c98/attachment-0001.html>