<p> Attention is currently required from: Kfir Itzhak. </p>
<p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/18008">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/a593a07c_4c3f573f">Patch Set #2, Line 14:</a> <code style="font-family:monospace,monospace">It is a best-effort request. It works by signaling the</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you clarify what best-effort really means? Is it guaranteed, or can it not happen?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008?tab=comments">Patch Set #2:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">This also needs a CHANGES[1] entry.</p><p style="white-space: pre-wrap; word-wrap: break-word;">[1] https://wiki.asterisk.org/wiki/display/AST/CHANGES+and+UPGRADE.txt</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File apps/app_queue.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/8cf4ed50_cab3f3ff">Patch Set #2, Line 301:</a> <code style="font-family:monospace,monospace">                               </variable></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The QUEUE_WITHDRAW_INFO dialplan variable should be documented here.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/465f72b0_50a5a215">Patch Set #2, Line 1713:</a> <code style="font-family:monospace,monospace">       char* withdraw_info;                   /*!< Optional info passed by the caller of QueueWithdrawCaller */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">char *withdraw_info;</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/4246d0a4_c0035f06">Patch Set #2, Line 7645:</a> <code style="font-family:monospace,monospace">             if (strcmp(ast_channel_name(qe->chan), caller) == 0) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Please review the coding guidelines[1].</p><p style="white-space: pre-wrap; word-wrap: break-word;">[1] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/67f1a9c1_844eb834">Patch Set #2, Line 7648:</a> <code style="font-family:monospace,monospace">                 if(withdraw_info && !qe->withdraw_info) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In what case would withdraw_info be set already?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/58a2136a_461313d3">Patch Set #2, Line 10805:</a> <code style="font-family:monospace,monospace">       switch (request_withdraw_caller_from_queue(queuename, caller, withdraw_info)) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18008">change 18008</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18008"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: Ic15aa238e23b2884abdcaadff2fda7679e29b7ec </div>
<div style="display:none"> Gerrit-Change-Number: 18008 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Kfir Itzhak <mastertheknife@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Kfir Itzhak <mastertheknife@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 10 Feb 2022 16:23:19 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>