Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test and fix for second grandchild bug. #49

Merged
merged 2 commits into from
May 23, 2013

Conversation

CrazyCasta
Copy link
Contributor

Bug is, second grandchild of a given child will get cast to the child
rather then the grandchild.

Bug is, second grandchild of a given child will get cast to the child
rather then the grandchild.
@CrazyCasta
Copy link
Contributor Author

Just FYI, I haven't been able to run runtest.sh so I don't have coverage information for this yet. I'll be happy to run coverage as soon as someone can explain what's wrong (from #42).

@CrazyCasta
Copy link
Contributor Author

Okay and the build failed because it timed out downloading some stuff so if someone could show me how to restart the build, that would be much appreciated too.

@CrazyCasta
Copy link
Contributor Author

I'm at an evaluation point now. It all looks good to me, but I want to point out that the behavior of select_subclasses is changed in Django >= 1.6.0 when a grandchild class is specified. Previously both the child and grandchild would be automatically subclassed when only the grandchild was specified. Now only the grandchild is subclassed. I would argue that this is better because it allows the programmer to more specifically choose what classes to subclass (which is why they were specifying them specifically after all).

Specifically the change in the test is to the test_select_specific_grandchildren function:

-                    self.child1,
+                    InheritanceManagerTestParent(pk=self.child1.pk), 

@carljm
Copy link
Collaborator

carljm commented May 23, 2013

This all looks great to me, and I agree with that behavior change. I'm not concerned about backwards-compatibility since so few people are likely to be using model-utils with grandchildren at all at this point. Merging!

@carljm carljm merged commit 6409799 into jazzband:master May 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants