<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/1341/">https://reviewboard.asterisk.org/r/1341/</a>
     </td>
    </tr>
   </table>
   <br />





 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">But there are still issues to be considered here.

Currently, since this change doesn&#39;t send a display name when there is no caller id, certain tests are bouncing mostly revolving around CDR expectation of caller id.
This failure in particular is in many of the CDR tests:

CDR MATCH FAILED, Expected: callerid:&quot;asterisk&quot; &lt;asterisk&gt; Got: callerid:asterisk

Right now, I&#39;m currently considering whether we should be stripping the display name if we have neither a name or a number.  That would probably fix these problems, but I&#39;m not sure whether the issue would be resolved by that change.  Changing the tests is another option, but I&#39;m still unsure if that&#39;s the only impact, and in addition, there would be different expected behavior between Asterisk 10 and before from Asterisk 11 in that case... which is awkward and makes the tests harder to maintain.

A number of tests remain that still need to be ran, so it&#39;s going to take a while to know the full impact.</pre>
 <br />







<p>- jrose</p>


<br />
<p>On August 3rd, 2011, 9:57 a.m., jrose 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, David Vossel and rmudgett.</div>
<div>By jrose.</div>


<p style="color: grey;"><i>Updated Aug. 3, 2011, 9:57 a.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;">This is basically what I was trying to do with the user submitted patch from yesterday.  After speaking with Richard, it became somewhat obvious that the approach to the patch had too much extra impact on the SIP messages when all that was really needed was specifically targeting the creation of the display name.

This approach checks strictly for a caller id name and if it isn&#39;t there, we skip the creation of the display name.  I think this should be safer in general than the previous approach.</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;">Testing was similar to last time with focus on comparing the SIP messages between this branch and the normal trunk.  In this case, only the display name gets changed.

A few SIP tests from the test suite were also ran successfully.  I haven&#39;t ran the full test suite yet, but am preparing to do so.</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/ASTERISK-16198">ASTERISK-16198</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>/trunk/CHANGES <span style="color: grey">(330571)</span></li>

 <li>/trunk/channels/chan_sip.c <span style="color: grey">(330571)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/1341/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>