| This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/17/ |
I had a couple of questions and some minor concerns but it looks pretty good overall.
| blockerbugs/controllers/main.py (Diff revision 1) | |||
|---|---|---|---|
| 292 | # app.logger.debug('bugid: %i' % bugform.bugid.data) |
293 | def store_bz_user(fas_login, bz_user): |
How come you broke this up as a separate function? It seems a bit stylistically different from the rest of the code
| blockerbugs/models/userinfo.py (Diff revision 1) | |||
|---|---|---|---|
| 28 | fas_login = db.Column(db.String(255), unique=True) |
||
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
| blockerbugs/templates/fas_bugzilla.html (Diff revision 1) | |||
|---|---|---|---|
| 30 | <form method="POST" action="{{ url_for('main.fas_bugzilla') }}"> |
||
Stylistically, this doesn't match the existing login page that we have for FAS - it would be better if they matched
| blockerbugs/util/bz_interface.py (Diff revision 1) | |||
|---|---|---|---|
def propose_bugs(self, bz_user, milestone_name, justification): |
|||
| 190 | raise |
||
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
- Tim
On March 13th, 2013, 10:32 a.m. UTC, Martin Krizek wrote:
|
Review request for blockerbugs.
By Martin Krizek.
Updated March 13, 2013, 10:32 a.m.
Bugs:
347
Repository:
blockerbugs
Description
Diffs
|