This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/17/

On March 13th, 2013, 9:32 p.m. UTC, Tim Flink wrote:

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
Initially, there were some checks that were not necessary in the end. I moved the code out of the function. Good point.

On March 13th, 2013, 9:32 p.m. UTC, Tim Flink wrote:

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
First I thought that usernames are of an e-mail form (which is not true) so that's why I put '255'. However I think it's safe to assume no one will have username longer than 255 characters, right?

On March 13th, 2013, 9:32 p.m. UTC, Tim Flink wrote:

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
Fair enough, fixed.

On March 13th, 2013, 9:32 p.m. UTC, Tim Flink wrote:

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
So I put try/catch block in the controller, let me know what you think.

- Martin


On March 14th, 2013, 11:24 a.m. UTC, Martin Krizek wrote:

Review request for blockerbugs.
By Martin Krizek.

Updated March 14, 2013, 11:24 a.m.

Bugs: 347
Repository: blockerbugs

Description

This is a patch for tickets 347, 348, 349 and 350.

Diffs

  • testing/test_bugchange.py (65f291122b4ae687872170517d196a621d548152)
  • blockerbugs/util/bz_interface.py (a8958be674e839d6c99694e33440a26331a8b041)
  • blockerbugs/templates/propose_bug.html (c97d72303dff2f49e4bb8bc5ff5fc2f4c764ce8b)
  • blockerbugs/templates/login.html (20674fe93eb96c6e1e9b12b5988cad67dc1a5181)
  • blockerbugs/templates/layout.html (6d0ae9a7bc913aa408985028db4494d51379da9c)
  • blockerbugs/templates/fas_bugzilla.html (PRE-CREATION)
  • blockerbugs/templates/base_nav.html (ce7a7bb478ad57a65d646fa7326a210c6a4eb131)
  • blockerbugs/models/userinfo.py (PRE-CREATION)
  • blockerbugs/controllers/users.py (30b5fb5c8ef2196979a2d22c4380f302a5ce951a)
  • blockerbugs/controllers/main.py (ce26c641b9db05f11510f16177b017a56cf757c2)
  • blockerbugs/controllers/forms.py (b213c055d9297b59c6cf5d83358dcd14e265e345)
  • alembic/versions/1d12b74d12bd_add_userinfo_table.py (PRE-CREATION)

View Diff