On Wed, Aug 7, 2013 at 4:03 AM, Christopher Medrela <[email protected] > 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 [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
