Code review comment for lp:~dholbach/harvest/schema-changes

Revision history for this message
James Westby (james-w) wrote :

Hi,

Looks ok to me.

Two comments though:

38 +class Person(models.Model):
39 + lpid = models.TextField(_("Who"), max_length=80)

is that long enough?

70 + if form.cleaned_data["comment"] != opportunity.comment:
71 + models.log_action(request.user.username,
72 + action="changed comment to: '%s'" % \
73 + form.cleaned_data["comment"],
74 + opportunity=opportunity)

that will overflow

    action = models.TextField(_("Action"), max_length=120)

if the comment is long won't it?

You probably want to ellipsise the comment in the action if it is
long.

Thanks,

James

review: Needs Fixing

« Back to merge proposal