<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="http://reviewboard-tflink.rhcloud.com/r/17/">http://reviewboard-tflink.rhcloud.com/r/17/</a>
</td>
</tr>
</table>
<br />
<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 had a couple of questions and some minor concerns but it looks pretty good overall.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://reviewboard-tflink.rhcloud.com/r/17/diff/1/?file=232#file232line293" style="color: black; font-weight: bold; text-decoration: underline;">blockerbugs/controllers/main.py</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">292</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c"># app.logger.debug('bugid: %i' % bugform.bugid.data)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">293</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">def</span> <span class="nf">store_bz_user</span><span class="p">(</span><span class="n">fas_login</span><span class="p">,</span> <span class="n">bz_user</span><span class="p">):</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">How come you broke this up as a separate function? It seems a bit stylistically different from the rest of the code</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://reviewboard-tflink.rhcloud.com/r/17/diff/1/?file=234#file234line28" style="color: black; font-weight: bold; text-decoration: underline;">blockerbugs/models/userinfo.py</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">28</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">fas_login</span> <span class="o">=</span> <span class="n">db</span><span class="o">.</span><span class="n">Column</span><span class="p">(</span><span class="n">db</span><span class="o">.</span><span class="n">String</span><span class="p">(</span><span class="mi">255</span><span class="p">),</span> <span class="n">unique</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">do we know if there are limits on the possible length of these usernames? I assume that there are but then again, I made the same assumption about other fields as well</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://reviewboard-tflink.rhcloud.com/r/17/diff/1/?file=236#file236line30" style="color: black; font-weight: bold; text-decoration: underline;">blockerbugs/templates/fas_bugzilla.html</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">30</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nt"><form</span> <span class="na">method=</span><span class="s">"POST"</span> <span class="na">action=</span><span class="s">"{{ url_for('main.fas_bugzilla') }}"</span><span class="nt">></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Stylistically, this doesn't match the existing login page that we have for FAS - it would be better if they matched</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://reviewboard-tflink.rhcloud.com/r/17/diff/1/?file=240#file240line190" style="color: black; font-weight: bold; text-decoration: underline;">blockerbugs/util/bz_interface.py</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; ">def propose_bugs(self, bz_user, milestone_name, justification):</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">190</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">raise</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If we get here, the app will blow up with an unhandled exception which originates from my code but as I'm looking at it again, I'm wondering about having a try/catch in the controller which sets an error in the proposal form and lets the user re-submit</pre>
</div>
<br />
<p>- Tim</p>
<br />
<p>On March 13th, 2013, 10:32 a.m. UTC, Martin Krizek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard-tflink.rhcloud.com/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 blockerbugs.</div>
<div>By Martin Krizek.</div>
<p style="color: grey;"><i>Updated March 13, 2013, 10:32 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://fedorahosted.org/fedora-qa/ticket/347">347</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
blockerbugs
</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;">This is a patch for tickets 347, 348, 349 and 350.</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>testing/test_bugchange.py <span style="color: grey">(65f291122b4ae687872170517d196a621d548152)</span></li>
<li>blockerbugs/util/bz_interface.py <span style="color: grey">(a8958be674e839d6c99694e33440a26331a8b041)</span></li>
<li>blockerbugs/templates/propose_bug.html <span style="color: grey">(c97d72303dff2f49e4bb8bc5ff5fc2f4c764ce8b)</span></li>
<li>blockerbugs/templates/login.html <span style="color: grey">(20674fe93eb96c6e1e9b12b5988cad67dc1a5181)</span></li>
<li>blockerbugs/templates/layout.html <span style="color: grey">(6d0ae9a7bc913aa408985028db4494d51379da9c)</span></li>
<li>blockerbugs/templates/fas_bugzilla.html <span style="color: grey">(PRE-CREATION)</span></li>
<li>blockerbugs/templates/base_nav.html <span style="color: grey">(ce7a7bb478ad57a65d646fa7326a210c6a4eb131)</span></li>
<li>blockerbugs/models/userinfo.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>blockerbugs/controllers/users.py <span style="color: grey">(30b5fb5c8ef2196979a2d22c4380f302a5ce951a)</span></li>
<li>blockerbugs/controllers/main.py <span style="color: grey">(ce26c641b9db05f11510f16177b017a56cf757c2)</span></li>
<li>blockerbugs/controllers/forms.py <span style="color: grey">(b213c055d9297b59c6cf5d83358dcd14e265e345)</span></li>
<li>alembic/versions/1d12b74d12bd_add_userinfo_table.py <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://reviewboard-tflink.rhcloud.com/r/17/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>