Hi Chris, On Wed, Aug 28, 2013 at 5:16 AM, Christopher Medrela < chris.medr...@gmail.com> wrote:
> 1. One of my questions left unanswered on the pull request [1] (I mean > this one > about documentation and `__str__` use.). > I've left a comment on the pull request, I've given the same comment in a previous message on this thread. Short version; I think there's potential for other levels to be used. (that said.. .I get the feeling I think I might have commented on the wrong comment -- it wasn't completely clear from the link you provided) 2. I've finished rewriting admin checks. I've renamed `admin_validation` to > `admin_checks`. I would like you to have a deep look at `fk_name` and > `exclude` checks [2] as well as `inline_formsets` tests [3] (especially > error > messages). > I'm not sure I follow what you've done with the exclude and fk_name checks here -- it looks like you've added some new checks, but I can't make out exactly why those checks are needed. I'm also not wild about the fact you've changed forms code along the way. 2a. "applabel.modellabel[.fieldname]" is the format of error messages for > models > and fields. How should the one for admin look like? I propose > "applabel.ModelAdminClassName". > This sounds reasonable to me; the only other idea I could suggest would be to include a '.admin' namespace in there, so it's clear that the error is part of an admin registration, but I'm not convinced that's needed. > 2b. BaseModelAdmin, ModelAdmin and InlineModelAdmin classes and their > checks > live in separated files. `options.py` defines these classes, but checks > lives > in `checks.py`. We want to have these two issues separated, otherwise class > definitions would become too long. Python does not support open classes, > so we > cannot just add some `_check_*` methods in `checks.py` to existing classes > defined in `options.py`. > > The current approach is that `check` method is defined in `options.py`, > but it > delegates to appropriate functions in `checks.py` [4]. Yes, I use > functions -- > there is no need to have validator class, because we don't need to store > anything in its instances. However, the code is really ugly. I wonder if > there > is any better approach. > Yes, there is. It's called object orientation and classes :-) The existing validation code already does this fairly elegantly - the only difference for your code is that you need to return multiple errors, not just raise an exception. I'm not sure why you've tried to re-invent this particular wheel. Russ %-) -- 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.
