Code review comment for lp:~acoconut/systers/flexible_essays

Revision history for this message
Robin J (robin-jeffries) wrote :

for this line
              if (question[2] != 'checkbox'):

This implies that you want to do this for every kind of widget except checkboxes. No, you are doing it for every kind of widget you currently have except checkboxes. Make it clearer for the next person and have it say
        if (question[2] == 'small text' or question[2] == 'large text'):

you also want to rethink your structure a bit more -- you have two tests for not being a checkbox, and you should be able to combine them into 1. (again, my guess is that if we add new widgets that are not texty, you may want to treat them differently, so explicitly list the question types you are using.

If these lines aren't useful to you, delete them; it's no longer accurate since you changed the utf-8
 command = "self.store.find(Subscriber,Subscriber.mailman_key == unicode(key,'utf-8')).set(deleted = False)"
           if DEBUG_MODE:
              syslog('info', 'DlistUtils(subscribeToList):Executing query:\n%s', command)

You defined acceptAnswer, but earlier you have almost identical code, why? Why not call acceptAnswer there?

And why aren't these identical? why do you have
    unicode_key = unicode(key, "utf-8")
in one place and
    userID = addr.decode('utf-8')
in the other

review: Needs Fixing

« Back to merge proposal