[asterisk-dev] [Code Review] Add option to prevent SIP diversion headers from being sent

[Mark Michelson]

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


The work here is good. The only suggestion I have is that I would much prefer if the option were not negatively-worded. So instead of an option to disable sending diversion, have an option to enable sending diversion. It will default to true, but people can change it to false if they don't want to send diversion headers. This in general makes things less confusing for all involved. A name like "send_diversion" (preferred) or "senddiversion" (not as preferred) would be good here.


/trunk/configs/sip.conf.sample
<https://reviewboard.asterisk.org/r/1769/#comment10264>

    This extra "Note" sentence isn't really needed and is littered with grammatical bugs.


- Mark


On Feb. 23, 2012, 10:36 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1769/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2012, 10:36 a.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> A while back, when 1.6.3 was a thing, A largish patch was introduced that enabled SIP diversion headers to be sent, presumably for call forwarding.  This adds an option to keep diversion headers introduced with that patch from being added to SIP dialogs because of interoperability issues that were introduced with that support.
> 
> 
> This addresses bug ASTERISK-16862.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16862
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 355731 
>   /trunk/channels/chan_sip.c 355731 
>   /trunk/channels/sip/include/sip.h 355731 
>   /trunk/configs/sip.conf.sample 355731 
> 
> Diff: https://reviewboard.asterisk.org/r/1769/diff
> 
> 
> Testing
> -------
> 
> I actually couldn't find a useful test scenario.  Call forwarding is a pretty roughly defined feature that tends to be implemented in the dialplan, and I couldn't find a method for call forwarding that actually included the Diversion header.  Still, the approach is really simple.  I don't think it'll be a problem.
> 
> 
> Thanks,
> 
> jrose
> 
>

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