<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1653/">https://reviewboard.asterisk.org/r/1653/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 18th, 2012, 5:05 p.m., <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You should not have added all the whitespace and curly brace cleanup changes to locations that you are not needing to change. They make an already large patch unnecessarily huge.
Also tabs should be set to 4 not 8.
I will continue to look at your patches.</pre>
</blockquote>
<p>On July 19th, 2012, 8:32 a.m., <b>KNK</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Initially i have not touched whitespaces and braces, but after each change i got new red blocks nearby, so i just went ahead and fixed all surrounding places.
The location changes were made in order to make 1.0 and (ex)trunk compatible in order to keep the NEW_API branch to be able to receive new changes and not to loose it's functionality for the future. A similar patch against (ex)trunk will be provided in the issue.
I reconfigured my editor with tabs set to 4, but i am not sure, should i realign the code now (as i can see it is mostly in headers) or post another patch later with additional curly brace cleanups too, which will be much easier to review?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Code cleanup patches that are just whitespace and curly braces should be done as a separate patch. They add a lot of noise when mixed with other types of changes. It is usually best to leave the code as is until you have some other reason to modify the function.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On July 13th, 2012, 2:41 p.m., KNK wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By KNK.</div>
<p style="color: grey;"><i>Updated July 13, 2012, 2:41 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">libss7 part of the changes.
Added additional cause codes, Transmission Medium Requirement setting and connected line to CPG messages + code cleanup.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">compiles, link setup, cli commands, bassic calls, connected line and redirection</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/SS7-21">SS7-21</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-27">SS7-27</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-28">SS7-28</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-33">SS7-33</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-36">SS7-36</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-38">SS7-38</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-39">SS7-39</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-40">SS7-40</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-42">SS7-42</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-43">SS7-43</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-45">SS7-45</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-46">SS7-46</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-47">SS7-47</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-48">SS7-48</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-49">SS7-49</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-51">SS7-51</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-52">SS7-52</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-53">SS7-53</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-54">SS7-54</a>,
<a href="https://issues.asterisk.org/jira/browse/SS7-7">SS7-7</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/1.0/ss7_internal.h <span style="color: grey">(300)</span></li>
<li>branches/1.0/ss7linktest.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/libss7.h <span style="color: grey">(300)</span></li>
<li>branches/1.0/mtp2.h <span style="color: grey">(300)</span></li>
<li>branches/1.0/mtp2.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/mtp3.h <span style="color: grey">(300)</span></li>
<li>branches/1.0/mtp3.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/parser_debug.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/ss7.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/isup.c <span style="color: grey">(300)</span></li>
<li>branches/1.0/isup.h <span style="color: grey">(300)</span></li>
<li>branches/1.0/ss7test.c <span style="color: grey">(300)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1653/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>