[asterisk-dev] [Code Review] 3193: cli: pjsip show endpoint <endpoint> shows allow/disallow codecs the same - OPTION 2

[Matt Jordan]

> On Feb. 6, 2014, 1:50 p.m., Joshua Colp wrote:
> > I like the idea of being able to mark stuff as deprecated and get a warning, but I still dislike the idea of hidden fields. ^_^

Two things here:

(1) There is already support for deprecated fields and information hiding in config_options. As sorcery is, for the most part, built on that framework, I would have expected to see those APIs being used, and not a re-creation of them. I suspect that there is a large amount of the implementation in here that could be simplified as a result.

(2) For the problem being solved here, this feels like overkill. If a consumer wants to hide information from the user, then the consumer should choose to hide that information. A framework should only hide that information when it is absolutely sure that no consumer, ever, in any module ever created (or that ever will be created), should have access to that information. I don't think that is the case here with 'disallow'.


- Matt


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


On Feb. 6, 2014, 12:50 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3193/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2014, 12:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
>     https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow                      : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it from a configuration file.
> 
> This is OPTION 2 - option 1 is https://reviewboard.asterisk.org/r/3136/
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_outbound_registration.c 407566 
>   /branches/12/res/res_pjsip_endpoint_identifier_ip.c 407566 
>   /branches/12/res/res_pjsip_acl.c 407566 
>   /branches/12/res/res_pjsip/pjsip_options.c 407566 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407566 
>   /branches/12/res/res_pjsip/location.c 407566 
>   /branches/12/res/res_pjsip/config_transport.c 407566 
>   /branches/12/res/res_pjsip/config_system.c 407566 
>   /branches/12/res/res_pjsip/config_global.c 407566 
>   /branches/12/res/res_pjsip/config_domain_aliases.c 407566 
>   /branches/12/res/res_pjsip/config_auth.c 407566 
>   /branches/12/main/sorcery.c 407566 
>   /branches/12/main/bucket.c 407566 
>   /branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3193/diff/
> 
> 
> Testing
> -------
> 
> Ran command and checked output.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140206/8512f273/attachment-0001.html>