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

Matt Jordan reviewboard at asterisk.org
Wed Oct 31 08:35:25 CDT 2012



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

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.


- Matt


-----------------------------------------------------------
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/79205ab5/attachment.htm>


More information about the asterisk-dev mailing list