[asterisk-dev] [Code Review]: SIP session timer tests for Require: timer header

[Mark Michelson]

> On Oct. 31, 2012, 2:45 a.m., Olle E Johansson wrote:
> > 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.
> 
> Matt Jordan wrote:
>     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.

@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.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2173/#review7341
-----------------------------------------------------------


On Oct. 30, 2012, 6:48 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2173/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 6:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> These are accompanying tests for the changes in https://reviewboard.asterisk.org/r/2172
> 
> 
> This addresses bug ASTERISK-20570.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20570
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-no-refresher.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-no-timer-support.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-refresher-uac.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/sipp/uac-refresher-uas.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/session_timers_require/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 3490 
> 
> Diff: https://reviewboard.asterisk.org/r/2173/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121031/ffbdae3c/attachment.htm>