<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1:</p><p style="white-space: pre-wrap; word-wrap: break-word;">Just a heads up (I haven't reviewed this fully) but this is not suitable for inclusion into 16 or 18 as-is. This changes the ast_transfer API in a breaking fashion.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Thank you Joshua.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I believe I modified everywhere ast_transfer was called to use the additional parameter. Is it unacceptable to do this?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I believe I could make it work without the additional parameter. It would require setting the return value to the error. Then, in code that calls ast_transfer, it would need to process the result (checking for anything 300+). Basically, setting it's process error value and change the original value to AST_SUCCESS/AST_FAILURE. Would this be a more acceptable approach (provided it's well documented)?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">We don't change APIs in a manner that breaks ABI compatibility in release branches, unless absolutely necessary to fix something like a security issue or other serious issue.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think changing the return value is acceptable, as while ABI compatible that's still a change to the API and a behavior change.</p><p style="white-space: pre-wrap; word-wrap: break-word;">A second API call being added is perfectly fine, however, as long as the existing one remains and continues to function.</p></blockquote><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15368">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15368/1/include/asterisk/channel.h">File include/asterisk/channel.h:</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/+/15368/1/include/asterisk/channel.h@2578">Patch Set #1, Line 2578:</a> <code style="font-family:monospace,monospace"> * \param protocol result code for transfer (0 success, else sip error code)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is supposed to not be protocol specific, so SIP can be mentioned as an example</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15368/1/main/channel.c">File main/channel.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/+/15368/1/main/channel.c@6515">Patch Set #1, Line 6515:</a> <code style="font-family:monospace,monospace"> enum ast_control_transfer *message = fr->data.ptr;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is a change to ast_control_transfer so that needs to be documented as well. Specifically that the message can contain a protocol specific code, and detail the acceptable range.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15368">change 15368</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/+/15368"/><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: Ia6a94784b4925628af122409cdd733c9f29abfc4 </div>
<div style="display:none"> Gerrit-Change-Number: 15368 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Dan Cropp <dan@amtelco.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-CC: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 22 Jan 2021 15:46:16 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>