[Asterisk-code-review] chan_dahdi: Add DAHDIBusyOut (asterisk[master])

[N A]

Attention is currently required from: Mark Murawski.

N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19264 )

Change subject: chan_dahdi: Add DAHDIBusyOut
......................................................................


Patch Set 5: Code-Review-1

(11 comments)

Patchset:

PS5: 
Across the board, the whitespace is all still messed up


File channels/chan_dahdi.h:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/52bfcefb_e59d1e99 
PS5, Line 206:        /*!
You're still using spaces here, not tabs, and the alignment is off


File channels/chan_dahdi.c:

https://gerrit.asterisk.org/c/asterisk/+/19264/comment/a509ae78_9105b036 
PS5, Line 211:        <manager name="DAHDIBusyOut" language="en_US">
Spaces should be tabs


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/7ac62c10_f946890e 
PS5, Line 225:                        <para>Enable or disable BusyOut on DAHDI channels.</para>
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.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/1c94d131_1235c729 
PS5, Line 227:                </description>
I don't see the xmldocs for the application, just the manager action.
Also, both of them should have see-also references to the other one.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ab028a52_88c956cb 
PS5, Line 5894: 
And all these spaces need to be converted to tabs too


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/a5aae23a_7b1db0d5 
PS5, Line 9786: 
I still think this is suspect.
For now, there should at least be a <note> in the xmldocs saying the channel will be answered.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ef149094_3427cf86 
PS5, Line 9788: 			ast_log(LOG_WARNING, "PBX exited non-zero\n");
This warning message is not accurate.


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/eaaeea88_079fd66e 
PS5, Line 11973: 
This is that blank line you added that should be removed in the change


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/b6a87d14_398e2a14 
PS5, Line 20289: static void *dahdi_set_channel_busyout_on_thread(void *data);
Why do you have a forward declaration when the definition is immediately below?


https://gerrit.asterisk.org/c/asterisk/+/19264/comment/dca9c4bf_21e8b7d7 
PS5, Line 20443: 	while (*channel_arg_cur);
The while should be on the same line as the end brace



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

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