<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/1713/">https://reviewboard.asterisk.org/r/1713/</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 3rd, 2012, 11:41 a.m., <b>Paul Belanger</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;">should't this be for trunk since this is a new feature?</pre>
</blockquote>
<p>On February 3rd, 2012, 12:01 p.m., <b>Mark Michelson</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;">From the linked issue, Leif left this comment:
"It's true that res_fax.so does not have 'reload' support. Per kpfleming and tlesher, if the module has configuration options, it is expected behaviour that we would support reloads, so to close this issue we need to add reload support.
For now, the work around is to unload and load the module to cause changes to happen."
It sounds as though this was discussed previously and determined to be a bug, rather than a feature request.</pre>
</blockquote>
<p>On February 3rd, 2012, 12:16 p.m., <b>Paul Belanger</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;">Understood, but adding a new CLI command within a release branch seems like a new feature. Like the original issue says, a workaround is to load / unload the module.
*shurgs*
It would be nice to define things like this in a wiki page, and consult it.</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 don't understand the confusion here; 'unload/load' of a module is a difficult operation to arrange on a busy system where FAX functionality is in use. If 'unload/load' was an acceptable alternative, it would be acceptable across the board, and we wouldn't put 'reload' support into any module.
As has already been commented here, if a module has configuration of its own, then it must support 'reload', unless doing so is impractical or problematic. Not doing so is a bug. Also, to be pedantic, this patch does not 'add a new CLI command'. It makes 'module reload' work for res_fax.</pre>
<br />
<p>- Kevin</p>
<br />
<p>On February 3rd, 2012, 11:26 a.m., Mark Michelson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Mark Michelson.</div>
<p style="color: grey;"><i>Updated Feb. 3, 2012, 11:26 a.m.</i></p>
<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;">Since res_fax has configurable options, it needs reload support. This patch adds reload support to res_fax.</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;">Confirmed that "module reload res_fax.so" is operational and properly loads new configuration options.</pre>
</td>
</tr>
</table>
<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-16712">ASTERISK-16712</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/res/res_fax.c <span style="color: grey">(353917)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1713/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>