On Wed, Aug 7, 2013 at 4:03 AM, Christopher Medrela <chris.medr...@gmail.com > wrote:
> I'm still working at polishing after reviewing. I've deprecated > `requires_model_validation` and `validate`. I've started at adding tests > for > contenttype fields: `GenericForeignKey` and `GenericRelation`. > > I've updated gsoc2013-checks-review branch [1]. Now it's the same as > gsoc2013-checks branch [2]. I will push new work to the latter while the > former > will remain unchanged. I'm working at contenttype tests in > gsoc2013-checks-contenttypes [3] branch. The work is not finished yet and > there > are some failing tests. > > @Shai Berger: Thank you for creating the ticket. I'm sorry that I > procrastinated accepting it -- I finally did it and proposed a patch. [4] > > [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks-review > [2] https://github.com/chrismedrela/django/tree/gsoc2013-checks > [3] > https://github.com/chrismedrela/django/tree/gsoc2013-checks-contenttypes > [4] https://code.djangoproject.com/ticket/20814 > > Questions: > > 1. Output formatting. We decided that every error/warning will take one > line plus > additional one for a hint if it's provided. The justification is that a > Django > user may type "grep HINT" to filter all hints. But now I think it's > unpractical > since the lines with hints doesn't say which object is invalid. So we can: > (1) > put hint in the same line as the error message or (2) change the format to > sth > like this: > > applabel.modellabel: Error message. > applabel.modellabel: HINT: Hint. > For my money, the label on the second line isn't needed. I know I gave grep as a use case, but in retrospect, that's probably isn't as useful as you'd think. The reason to have the newline break with an indent is so that theres a visual cue for any hint. It makes the hint stand out. Putting a prefix on that line obscures this. applabel.modellabel: Error message HINT: the hint > 2. Is it allowed to use `GenericRelation` pointing to a model if the model > lacks > `GenericForeignKey`? > In theory, I suppose you could, as long as there was a pair of object and content type fields. However, in practice, I don't think this is a very likely use case. I'd be completely comfortable if you confirmed the existence of a GenericForeignKey. Raising this as a warning might be a good approach here. 3. I've added unicode_literals import to django/core/management.py but this > affected `BaseCommand.verbosity` default value. In order not to break > commands, > I left the attribute as a bytestring [5]. Changing it to an unicode breaks > some admin_scipts tests, i. e. `CommandTypes.test_app_command` expects in > stdout: > > ... options=[('pythonpath', None), ... ('verbosity', '1')] > > while the command prints: > > ... options=[('pythonpath', None), ... ('verbosity', u'1')] > I can see what you're doing here, but I'm not sure it's needed. u'1' == '1' under Python 2.7, and under Python 3 it's all unicode anyway. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
