#5177: ContentType should purge orphaned models
---------------------------------------------------+------------------------
Reporter: Rob Hudson <[EMAIL PROTECTED]> | Owner: adrian
Status: new | Component:
Contrib apps
Version: SVN | Resolution:
Keywords: | Stage:
Accepted
Has_patch: 1 | Needs_docs: 1
Needs_tests: 0 | Needs_better_patch: 1
---------------------------------------------------+------------------------
Changes (by mtredinnick):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I like this patch and assuming all the concerns below are not real
problems, it's close to reday. A few small things to fix and one large
thing to ponder:
1. create_contenttypes() now really needs a docstring, because it's not
just creating new content types. It's also cleaning up old ones. Looks
like it's a bit fiddly to move the "remove" processing out from the
"create" loop, so it'll have to stay as one method. But at least document
what it's doing in a broad way.
2. We're strong believers in the English language and full sentences. The
comment you've got to needs an initial capital and final full stop.
3. The AUTHORS file is in alphabetical order by surname. Try to find
where your name belongs (hint: it's not at the end).
4. Do you need to get app_label from the models? What happens if a model
has Meta.app_label set?
Now here's the big problem: is this a safe thing to do? I haven't followed
through all the code, but I would guess it's not impossible for models in
different locations to have the same value for their Meta.app_label
attribute. I'm not sure if Django works well in that case, or if
django.core.management sorts that out and groups things properly (I
suspect not), or if it's something we want to support. But right now it
might be possible. If so, that would mean you could end up calling
create_contenttypes() multiple times for the same app_label.
At the moment, I suspect that just means not as much cleaning up gets
done, so it fails safely (no still-valid data is deleted), right? I think
this bears some thinking about; I don't know the answer yet, because it
just occurred to me when I was reviewing the patch.
Not sure it's going to really be possible to test this, since the
!ContentTypes table is flushed for each new test anyway. If you can come
up with something, great. However, I can't think of how to do it offhand.
What docs do you think are necessary? A quick observational note in the
contrib apps docs? It's almost doing the natural thing and could pass
without comment. By all means, write whatever you think is good, but I
wouldn't hold back the patch if you think there's nothing worth saying.
--
Ticket URL: <http://code.djangoproject.com/ticket/5177#comment:3>
Django Code <http://code.djangoproject.com/>
The web framework for perfectionists with deadlines
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
