<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/3136/">https://reviewboard.asterisk.org/r/3136/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 6th, 2014, 9:52 a.m. CST, <b>Joshua Colp</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;">While this works I'm not happy with pushing this down to such a low level. What if in the future I want to filter something else?
What I'd really like to see is something on top which allows you to arbitrarily filter anything. If something similar came up in the future then we'd have an immediate easy solution with no core changes and it would also allow the information to still exist for cases where you do want to get it.</pre>
</blockquote>
<p>On February 6th, 2014, 11:40 a.m. CST, <b>Scott Griepentrog</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;">The other option would be to add another parameter to the ast_sorcery_object_field_register() which would indicate a "hidden" option. I can write that up as a separate review and see which one is preferred. Which is now up for review: https://reviewboard.asterisk.org/r/3193/
</pre>
</blockquote>
<p>On February 6th, 2014, 11:43 a.m. CST, <b>Joshua Colp</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;">I disagree that that is the only other option. You don't have to consume the information provided from sorcery as-is, and you don't have to push filtering down to that level. Something that sits between sorcery and the consumer can easily do the filtering based on information the consumer provides.</pre>
</blockquote>
<p>On February 6th, 2014, 11:45 a.m. CST, <b>Joshua Colp</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;">To go a step further: Whether an object field should be hidden is not a property of the object field itself, it is a constraint within the consumer.</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;">I'm in 100% agreement with Josh here. I don't think this or option 2 are the right approach.
A framework should provide information to consumers. It is up to the consumer to decide how that information is displayed.
What do we mean by consumer?
Today, that is the CLI and AMI. But it doesn't have to be.
I could just as easily have a module that extracts information about SIP endpoints via a sorcery object and transmits that data over a JSON websocket (why, I don't know, but think of the integration!) Since it is not displaying information to an end user, it might want to see the disallow field. It might not: but the answer is, I don't know what it wants to send. If this becomes part of the framework, then it precludes this module from ever existing. You can't go back and simply 'undo' not showing the option - you've now broken the CLI/AMI. Your only option is to undo it *and* modify the CLI/AMI, and now that module has become intrusive.
I'd rather keep this sorcery agnostic of how things use it. Whenever possible, let the consumers decide what and how to show data. A filter is not a bad thing; it is appropriate for a consumer to filter out what it doesn't want to show.</pre>
<br />
<p>- Matt</p>
<br />
<p>On February 6th, 2014, 11:40 a.m. CST, Scott Griepentrog wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Scott Griepentrog.</div>
<p style="color: grey;"><i>Updated Feb. 6, 2014, 11:40 a.m.</i></p>
<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-23092">ASTERISK-23092</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Ran command and checked output.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/res/res_pjsip/pjsip_configuration.c <span style="color: grey">(407196)</span></li>
<li>/branches/12/main/sorcery.c <span style="color: grey">(407196)</span></li>
<li>/branches/12/main/config_options.c <span style="color: grey">(407196)</span></li>
<li>/branches/12/include/asterisk/config_options.h <span style="color: grey">(407196)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3136/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>