<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/2173/">https://reviewboard.asterisk.org/r/2173/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 31st, 2012, 2:45 a.m., <b>Olle E Johansson</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;">First, it would be nice to get some attribution here since I reported this bug in the existing code. Secondly, please take a look at my code for require in my PRACK branch.
I added a more generic way to support Require headers, since if we have both session timers and PRACK and possibly something else, the Require header needs to list all of them. Your patch just solves it for timer, but will cause issues if other extensions are required. Feel free to contact me if you need help finding it.
Basically I set flags for the required extensions, like we do with supported, then add the header in a generic way for all the required extensions.</pre>
</blockquote>
<p>On October 31st, 2012, 8:35 a.m., <b>Matt Jordan</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;">Issue reporters are rarely given attribution directly in the description on a code review, and there certainly is no policy to do so. (I'd be a little hesitatant to force that too - making code reviews more difficult isn't a goal of mine! :-))
Typically attribution for issue reporters is done via linking to the ASTERISK issue - which, in this case, since you informed me of the problem and I filed the issue, links to me as the reporter. Mark is aware that you were the one who found this particular problem, and I'm sure he'll attribute you as the reporter of the problem when the commit happens.
To make sure that happens however, I've gone ahead and made a note in the ASTERISK issue that you were the one who noticed this problem.</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;">@Olle: I'll have a look at your PRACK branch. You're right that if another Require header were required in a response, it would be a pain to try to get it listed given how I've done it. If I use the method from the PRACK branch, I'll be sure to give credit to you in the commit for that as well as for finding the problem in the first place.</pre>
<br />
<p>- Mark</p>
<br />
<p>On October 30th, 2012, 6:48 p.m., Mark Michelson 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 Mark Michelson.</div>
<p style="color: grey;"><i>Updated Oct. 30, 2012, 6:48 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;">These are accompanying tests for the changes in https://reviewboard.asterisk.org/r/2172</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-20570">ASTERISK-20570</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>/asterisk/trunk/tests/channels/SIP/session_timers_require/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/configs/ast1/sip.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-no-refresher.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-no-timer-support.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-refresher-uac.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-refresher-uas.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/session_timers_require/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/tests.yaml <span style="color: grey">(3490)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2173/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>