Hi Chris, On Wed, Aug 28, 2013 at 5:16 AM, Christopher Medrela < [email protected]> 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 [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.
