[Asterisk-code-review] pjsip/dialplan_functions.c: Get dialable string from function. (asterisk[master])

[Sean Bright]

Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/13349 )

Change subject: pjsip/dialplan_functions.c: Get dialable string from function.
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

Not sold on 'never_empty' but 'please' is not great. Someone else might have a better idea.

https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c 
File channels/pjsip/dialplan_functions.c:

https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c@47 
PS1, Line 47: 		<parameter name="please" required="false">
I'm anti-"please." How about 'never_empty' or something else that is more descriptive?


https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c@48 
PS1, Line 48: 			<para>Optional return of 'PJSIP/endpoint' if no contacts are found. Otherwise, with no contacts, this function will return an empty string that will cause the Dial application to exit with an error and hang up the call.</para>
This needs appropriate line wrapping.


https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c@1007 
PS1, Line 1007: 			ast_log(LOG_WARNING, "Specified endpoint '%s' was not found but the 'please' argument was provided so keeping calm and carrying on\n", args.endpoint_name);
If you've provided the argument, this is a non-exceptional situation so we don't need a warning. Drop it down to an ast_debug, and change the message to something like "... was provided so using PJSIP/%s"


https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c@1061 
PS1, Line 1061: 	if (ast_str_strlen(dial) == 0 && !ast_strlen_zero(args.please)) {
!ast_str_strlen(dial)


https://gerrit.asterisk.org/c/asterisk/+/13349/1/channels/pjsip/dialplan_functions.c@1062 
PS1, Line 1062: 		ast_log(LOG_WARNING, "No current Contacts found for endpoint '%s' but the 'please' argument was provided so keeping calm and carrying on\n", args.endpoint_name);
If you've provided the argument, this is a non-exceptional situation so we don't need a warning. Drop it down to an ast_debug, and change the message to something like "... was provided so using PJSIP/%s"



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I1709ee211705edac4d93f55723b8153bb653a338
Gerrit-Change-Number: 13349
Gerrit-PatchSet: 1
Gerrit-Owner: cmaj <chris at penguinpbx.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-CC: Friendly Automation
Gerrit-Comment-Date: Tue, 03 Dec 2019 22:03:27 +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/20191203/2a45c29e/attachment-0001.html>