[Asterisk-code-review] app_queue: Add QueueWithdrawCaller AMI action (asterisk[16])

[Joshua Colp]

Attention is currently required from: Kfir Itzhak.
Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18008 )

Change subject: app_queue: Add QueueWithdrawCaller AMI action
......................................................................


Patch Set 2: Code-Review-1

(7 comments)

Commit Message:

https://gerrit.asterisk.org/c/asterisk/+/18008/comment/a593a07c_4c3f573f 
PS2, Line 14: It is a best-effort request. It works by signaling the
Can you clarify what best-effort really means? Is it guaranteed, or can it not happen?


Patchset:

PS2: 
This also needs a CHANGES[1] entry.

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


File apps/app_queue.c:

https://gerrit.asterisk.org/c/asterisk/+/18008/comment/8cf4ed50_cab3f3ff 
PS2, Line 301: 				</variable>
The QUEUE_WITHDRAW_INFO dialplan variable should be documented here.


https://gerrit.asterisk.org/c/asterisk/+/18008/comment/465f72b0_50a5a215 
PS2, Line 1713: 	char* withdraw_info;                   /*!< Optional info passed by the caller of QueueWithdrawCaller */
char *withdraw_info;


https://gerrit.asterisk.org/c/asterisk/+/18008/comment/4246d0a4_c0035f06 
PS2, Line 7645: 		if (strcmp(ast_channel_name(qe->chan), caller) == 0) {
Please review the coding guidelines[1].

[1] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines


https://gerrit.asterisk.org/c/asterisk/+/18008/comment/67f1a9c1_844eb834 
PS2, Line 7648: 			if(withdraw_info && !qe->withdraw_info) {
In what case would withdraw_info be set already?


https://gerrit.asterisk.org/c/asterisk/+/18008/comment/58a2136a_461313d3 
PS2, Line 10805: 	switch (request_withdraw_caller_from_queue(queuename, caller, withdraw_info)) {
I think from a user perspective this should tell the user if the channel is already in the process of being withdrawn. I say this because right now if it is, then this becomes a no-op and this WithdrawInfo is just gone.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ic15aa238e23b2884abdcaadff2fda7679e29b7ec
Gerrit-Change-Number: 18008
Gerrit-PatchSet: 2
Gerrit-Owner: Kfir Itzhak <mastertheknife at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Kfir Itzhak <mastertheknife at gmail.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 16:23:19 +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/20220210/c216778e/attachment.html>