<p> Attention is currently required from: Martin McCarthy. </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/+/19939">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">File contrib/scripts/install_prereq:</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/+/19939/comment/17c10524_b545f5a9">Patch Set #2, Line 7:</a> <code style="font-family:monospace,monospace">installing</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Reformat for 80 cols</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/2fd7ca15_d49323ca">Patch Set #2, Line 9:</a> <code style="font-family:monospace,monospace"># - DragonFly</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Alphabetically sort this list</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/f479b7b2_7b3f31ca">Patch Set #2, Line 25:</a> <code style="font-family:monospace,monospace"># Function name: handle_debian</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The function name here is obvious.  Each comment block doesn't need to repeat the function name.  Same goes for all the others.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/cd7c71c8_71a29b7f">Patch Set #2, Line 26:</a> <code style="font-family:monospace,monospace"># Date: 01/11/22</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think it's necessary to date every function here.  This is not a library/api file.  Same goes for all the others.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/c58c50a2_d755fd73">Patch Set #2, Line 30:</a> <code style="font-family:monospace,monospace">    if ! [ -x "$(command -v aptitude)" ] ; then</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">All indenting needs to 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/+/19939/comment/e3787018_9f0e8a31">Patch Set #2, Line 41:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove unnecessary 'x'.  Straight string comparison is sufficient 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/+/19939/comment/ae88bd01_32ebace2">Patch Set #2, Line 48:</a> <code style="font-family:monospace,monospace">    # We're done!</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Don't need to write a 'done' comment for every return unless there's something special going on.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/cf91ba8e_0ddd83b9">Patch Set #2, Line 58:</a> <code style="font-family:monospace,monospace">    extra_packs=`check_installed_rpms $PACKAGES_RH`</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In the comment blocks for each function.  Document the prerequisites for the function.  Describe what $PACKAGES_RH is supposed to be prior to the handle_rh() call</p><p style="white-space: pre-wrap; word-wrap: break-word;">Same goes for all the other functions</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/77f7522c_7d4e18de">Patch Set #2, Line 60:</a> <code style="font-family:monospace,monospace">    if [ x"$extra_packs" != "x" ] ; then</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove unnecessary 'x'.  Straight string comparison is sufficient 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/+/19939/comment/b12b10ee_379eb977">Patch Set #2, Line 79:</a> <code style="font-family:monospace,monospace">    if [ x"$extra_packs" != "x" ] ; then</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same as above.  Please fix all of these.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19939/comment/baf4c874_a6dbd076">Patch Set #2, Line 303:</a> <code style="font-family:monospace,monospace">        </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Fix coding style</p><p style="white-space: pre-wrap; word-wrap: break-word;">; if [ condition ]; then<br>;   dostuff<br>; fi</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19939">change 19939</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/+/19939"/><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: I4bd53ae429e113a76d13a23b48da714a8eefad26 </div>
<div style="display:none"> Gerrit-Change-Number: 19939 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Martin McCarthy <martin.c.mccarthy@outlook.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-Attention: Martin McCarthy <martin.c.mccarthy@outlook.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 06 Mar 2023 18:47:30 +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>