I think establishing a policy of who is allowed to give a "ship it" is a very good idea. I do not think that being a Digium employee is the right criteria. There are people outside of Digium that should be able to ACK a patch. Also, a new hire at Digium may not be appropriate to give an ACK, either.<br>
<br>I would propose a 2-tier committers system:<br><br> - A committer (someone with commit access)<br> - A reviewer (a committer that can sign off on non-trivial changes on reviewboard)<br><br>Then you have to answer how one can become a committer or a reviewer. I'll kick off the topic with proposing:<br>
<br> - A contributer can become a committer if backed by a reviewer.<br> - A committer can become a reviewer if backed by 2 other reviewers.<br><br>Obviously everyone who already has commit access would be the committers. The initial reviewer pool would have to be seeded by project leadership (Kevin, with input from others, I suppose).<br>
<br clear="all">--<br>Russell Bryant<br>
<br><br><div class="gmail_quote">On Mon, Oct 24, 2011 at 5:28 PM, jrose <span dir="ltr"><<a href="mailto:reviewboard@asterisk.org">reviewboard@asterisk.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div>
<div style="font-family:Verdana, Arial, Helvetica, Sans-Serif">
<table style="border:1px #c9c399 solid" bgcolor="#f9f3c9" cellpadding="8" width="100%">
<tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1463/" target="_blank">https://reviewboard.asterisk.org/r/1463/</a>
</td>
</tr>
</tbody></table>
<br>
<blockquote style="margin-left:1em;border-left:2px solid #d0d0d0;padding-left:10px">
<p style="margin-top:0">On October 24th, 2011, 4:24 p.m., <b>jrose</b> wrote:</p>
<blockquote style="margin-left:1em;border-left:2px solid #d0d0d0;padding-left:10px">
<table style="border:1px solid #C0C0C0;border-collapse:collapse" bgcolor="white" border="0" width="100%">
<thead>
<tr>
<th colspan="4" style="border-bottom:1px solid #C0C0C0;font-size:9pt;padding:4px 8px;text-align:left" bgcolor="#F0F0F0">
<a href="https://reviewboard.asterisk.org/r/1463/diff/1/?file=20901#file20901line3911" style="color:black;font-weight:bold;text-decoration:underline" target="_blank">/trunk/main/features.c</a>
<span style="font-weight:normal">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color:#e4d9cb;padding:4px 8px;text-align:center">
<tr>
<td colspan="4"><pre style="font-size:8pt;line-height:140%;margin:0">int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct ast_bridge_config *config)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th style="border-right:1px solid #C0C0C0" align="right" bgcolor="#e9eaa8"><font size="2">3911</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>ast_dtmf_stream</span><span>(</span><span>chan</span><span>,</span> <span>peer</span><span>,</span> <span>peer_featurecode</span><span>,</span> <span>0</span><span>,</span> <span><span>0</span></span><span>);</span></pre>
</td>
<th style="border-left:1px solid #C0C0C0;border-right:1px solid #C0C0C0" align="right" bgcolor="#e9eaa8"><font size="2">3911</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>ast_dtmf_stream</span><span>(</span><span>chan</span><span>,</span> <span>peer</span><span>,</span> <span>peer_featurecode</span><span>,</span> <span>0</span><span>,</span> <span><span>f</span></span><span><span>-></span></span><span><span>len</span></span><span>);</span></pre>
</td>
</tr>
</tbody>
<tbody>
<tr>
<th style="border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3912</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>memset</span><span>(</span><span>peer_featurecode</span><span>,</span> <span>0</span><span>,</span> <span>sizeof</span><span>(</span><span>peer_featurecode</span><span>));</span></pre>
</td>
<th style="border-left:1px solid #C0C0C0;border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3912</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>memset</span><span>(</span><span>peer_featurecode</span><span>,</span> <span>0</span><span>,</span> <span>sizeof</span><span>(</span><span>peer_featurecode</span><span>));</span></pre>
</td>
</tr>
<tr>
<th style="border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3913</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>}</span></pre></td>
<th style="border-left:1px solid #C0C0C0;border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3913</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>}</span></pre></td>
</tr>
<tr>
<th style="border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3914</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>if</span> <span>(</span><span>!</span><span>ast_strlen_zero</span><span>(</span><span>chan_featurecode</span><span>))</span> <span>{</span></pre>
</td>
<th style="border-left:1px solid #C0C0C0;border-right:1px solid #C0C0C0" align="right" bgcolor="#f0f0f0"><font size="2">3914</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>if</span> <span>(</span><span>!</span><span>ast_strlen_zero</span><span>(</span><span>chan_featurecode</span><span>))</span> <span>{</span></pre>
</td>
</tr>
</tbody>
<tbody>
<tr>
<th style="border-right:1px solid #C0C0C0" align="right" bgcolor="#e9eaa8"><font size="2">3915</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>ast_dtmf_stream</span><span>(</span><span>peer</span><span>,</span> <span>chan</span><span>,</span> <span>chan_featurecode</span><span>,</span> <span>0</span><span>,</span> <span><span>0</span></span><span>);</span></pre>
</td>
<th style="border-left:1px solid #C0C0C0;border-right:1px solid #C0C0C0" align="right" bgcolor="#e9eaa8"><font size="2">3915</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>ast_dtmf_stream</span><span>(</span><span>peer</span><span>,</span> <span>chan</span><span>,</span> <span>chan_featurecode</span><span>,</span> <span>0</span><span>,</span> <span><span>f</span></span><span><span>-></span></span><span><span>len</span></span><span>);</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">f->len is not good here. Frame can very easily be null within this block of code, and if it is, trying to access f->len will cause a segfault... and in fact, messing about with google voice with the interns today exposed this very problem.
Generally speaking you need to wait for someone internal to give a ship-it before committing code if you have commit access. I don't actually know whether that status of people on reviewboard is actually viewable anywhere, so I guess for now the only real answer to that is that if you don't know, you need to ask on IRC. I'm going to go ahead and fix these items since it's a pretty easy change, but just keep that in mind int he future.</pre>
</blockquote>
</blockquote>
<pre style="margin-left:1em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">s/someone internal/Digium Developer
*in the future</pre>
<br>
<p>- jrose</p>
<br>
<p>On September 27th, 2011, 8:53 a.m., Olle E Johansson wrote:</p>
<table style="background-repeat:repeat-x;border:1px black solid" bgcolor="#fefadf" cellpadding="8" cellspacing="0" width="100%">
<tbody><tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Olle E Johansson.</div>
<p style="color:grey"><i>Updated Sept. 27, 2011, 8:53 a.m.</i></p>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" cellspacing="0" width="100%">
<tbody><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">When DTMF goes through features (because we're listening for some feature codes) the length is disregarded. This patch makes sure that the DTMF length doesn't disappear in a few cases.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" cellspacing="0" width="100%">
<tbody><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">This is part of a larger branch that fixes a lot of DTMF issues.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">
<li>/trunk/main/features.c <span style="color:grey">(338029)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1463/diff/" style="margin-left:3em" target="_blank">View Diff</a></p>
</td>
</tr>
</tbody></table>
</div>
</div>
<br>--<br>
_____________________________________________________________________<br>
-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com" target="_blank">http://www.api-digital.com</a> --<br>
<br>
asterisk-dev mailing list<br>
To UNSUBSCRIBE or update options visit:<br>
<a href="http://lists.digium.com/mailman/listinfo/asterisk-dev" target="_blank">http://lists.digium.com/mailman/listinfo/asterisk-dev</a><br></blockquote></div><br>