Add support for model packages #34
Conversation
register() will place records under given app. Meta app_label will place both model / records under given app.
|
@treyhunner, this seems good to merge, will wait for you to pull the trigger. |
|
I'm not sure this tests the same problem that #7 notes. It seems like a test of this would structure the models as: This app is structured this way: https://github.com/thauber/django-schedule/tree/master/schedule/models |
|
this is the same problem in fact. when models are defined in another module and get created from models.Model metaclass, it needs to specify app_label to go under the correct app. the tests previously had it a little backwards where ExternalModel1 / ExternalModel3 use app_label rather than mimic directory structure, same behavior however. |
| @@ -135,9 +143,10 @@ def get_meta_options(self, model): | |||
| Returns a dictionary of fields that will be added to | |||
| the Meta inner class of the historical record model. | |||
| """ | |||
| return { | |||
| options = { | |||
There was a problem hiding this comment.
What's the reasoning behind this style change?
PEP8 doesn't state an opinion on this, but I've always preferred return {...} over x = {...} ; return x.
There was a problem hiding this comment.
too many things at once, was looking at issue #26 but failed to revert that part.
|
It wasn't clear to me before that it was testing the same thing. It's definitely clear to me now. The tests seem to properly fail when the code change is reverted. This looks good for merging to me. |
Tests and code to fix #7.
Will be merged once working.