<p>Matthew Fredrickson <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/6173">View Change</a></p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c">File pbx/pbx_dundi.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c@253">Patch Set #1, Line 253:</a> <code style="font-family:monospace,monospace">    union ip_addr  addr;                            /*!< Other end of transaction */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Nitpick, but keeping indentation styles with the existing file is usually best practice when making a contribution to another project.  Asterisk generally indents using tabs, not spaces.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6173/1/pbx/pbx_dundi.c@439">Patch Set #1, Line 439:</a> <code style="font-family:monospace,monospace">        if ( netsock_ai_family == AF_INET6)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Another nitpick, following existing coding styles is usually best practice when contributing.  So typically, opening brackets for conditionals should be on the same lines as the conditional.  Also, preceding whitespace between the '(' character and the beginning of the conditional element "if ( netsock_ai_family" is not convention in Asterisk's code - instead should be "if (netsock_ai_family".</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">For example, it should probably look like this:<br>if (netsock_ai_family == AF_INET6) {<br>   ....<br>} else {<br>   ....<br>}</pre><p style="white-space: pre-wrap; word-wrap: break-word;">You actually might want to look at the coding guidelines wiki page at:<br>https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/6173">change 6173</a>. To unsubscribe, 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/6173"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Ifbe298afc6416ba400db7be404a25994ad23968a </div>
<div style="display:none"> Gerrit-Change-Number: 6173 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Adam Secombe <adam.j.secombe@boeing.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 28 Aug 2017 14:11:42 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>