<p> Attention is currently required from: Mark Murawski. </p>
<p>Patch set 5:<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/+/19264">View Change</a></p><p>11 comments:</p><ul style="list-style: none; padding: 0;"><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/+/19264?tab=comments">Patch Set #5:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Across the board, the whitespace is all still messed up</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File channels/chan_dahdi.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/+/19264/comment/52bfcefb_e59d1e99">Patch Set #5, Line 206:</a> <code style="font-family:monospace,monospace">       /*!</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You're still using spaces here, not tabs, and the alignment is off</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File channels/chan_dahdi.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/+/19264/comment/a509ae78_9105b036">Patch Set #5, Line 211:</a> <code style="font-family:monospace,monospace">       <manager name="DAHDIBusyOut" language="en_US"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Spaces should be tabs</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/7ac62c10_f946890e">Patch Set #5, Line 225:</a> <code style="font-family:monospace,monospace">                       <para>Enable or disable BusyOut on DAHDI channels.</para></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is not descriptive enough. What does enable or disable BusyOut mean? It should be an explanation that a new user would understand, what it does, how it works under the hood, and what the implications are. e.g. how is it busy, what will it do? etc.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/1c94d131_1235c729">Patch Set #5, Line 227:</a> <code style="font-family:monospace,monospace">               </description></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't see the xmldocs for the application, just the manager action.<br>Also, both of them should have see-also references to the other one.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ab028a52_88c956cb">Patch Set #5, Line 5894:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">And all these spaces need to be converted to tabs too</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/a5aae23a_7b1db0d5">Patch Set #5, Line 9786:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I still think this is suspect.<br>For now, there should at least be a <note> in the xmldocs saying the channel will be answered.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ef149094_3427cf86">Patch Set #5, Line 9788:</a> <code style="font-family:monospace,monospace">                    ast_log(LOG_WARNING, "PBX exited non-zero\n");</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This warning message is not accurate.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/eaaeea88_079fd66e">Patch Set #5, Line 11973:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is that blank line you added that should be removed in the change</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b6a87d14_398e2a14">Patch Set #5, Line 20289:</a> <code style="font-family:monospace,monospace">static void *dahdi_set_channel_busyout_on_thread(void *data);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why do you have a forward declaration when the definition is immediately below?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/dca9c4bf_21e8b7d7">Patch Set #5, Line 20443:</a> <code style="font-family:monospace,monospace">  while (*channel_arg_cur);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The while should be on the same line as the end brace</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19264">change 19264</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/+/19264"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ib75c87c554bb18b8f65be73b4474474e7e1877b7 </div>
<div style="display:none"> Gerrit-Change-Number: 19264 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Attention: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-Comment-Date: Sat, 17 Sep 2022 16:24:47 +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>