Code review comment for lp:~marco-gallotta/ibid/uva

Serious stuff:
--------------

What's with the modification to identity? I thought we needed to use a string.

Query: create acm team foo
./ibid/plugins/codecontest.py:409: SADeprecationWarning: Use session.add()
  event.session.save_or_update(team)
Response: Created ACM team foo
Query: acm team foo
ERROR:scripts.ibid-plugin:Exception occured in ACMTeamManagement processor of codecontest plugin
Traceback (most recent call last):
  File "scripts/ibid-plugin", line 140, in <module>
    processor.process(event)
  File "./ibid/plugins/__init__.py", line 119, in process
    method(event, *match.groups())
  File "./ibid/plugins/codecontest.py", line 505, in acmteam_info
    u'uva_teams': uva_teams,
  File "./ibid/event.py", line 53, in addresponse
    response = response % params
ValueError: incomplete format
WARNING:plugins.unicode:Found a non-unicode string: exception
Response: I'm not feeling too well

Stylistic section of the review:
--------------------------------

> event.addresponse(u'Done')
All of those kind of things should be replaced with either a more descriptive answer ("OK, Team created"), or simply addresponse(True).

> member = event.session.query(ACMTeamMember) \
> .filter(ACMTeam.name==team_name) \
> .filter_by(name=member_name) \
It'd read better if the two filter lines where the other way around, or you used filter() for both instead of one filter(), one filter_by().

> human_join(sorted([team.name for team in teams])))
You can use a generator expression (i.e. remove the square brackets)

> uva_teams = ', and is known as %s on UVa' % human_join(uva_teams)
> else:
> uva_teams = ''
Unicode

> u'%(team_name)s has member%(plural)s %(team_members)s%(uva_teams)s'
> ...
> u'plural': plural(len(team.members), '', 's')
Bad usage of plural. It should be plural(len(team.members), 'member', 'members'). There's another one further down.

> hours = time_penalty / 3600
> time_penalty -= hours * 3600
> minutes = time_penalty / 60
Use the integer division operator //, it makes intent clearer.

> class ACMProblemScore:
That should probably be a new-style class. Applies to the others, too.
Not that it matters :)

> def __str__(self):
Should probably be __unicode__, as it returns unicode. Also applies to the other class.

> def setup(self):
> super(Uva, self).setup()
Redundant.

> u"Sorry, I'm not monitoring the scoreboard anyway"
That needs to be reworded, it doesn't parse.