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

fix InheritedPropertyMetadataFactory to build correctly the metadata #2869

Closed
wants to merge 2 commits into from

Conversation

alessandroniciforo
Copy link

The InheritedPropertyMetadataFactory is supposed to build the metadata of all the inherited properties of a class from its parents. This was not working because of a mistake on the is_subclass_of check.
The related test was always passing because of a wrong mock. I fixed that, too.

There are some related issues that have been fixed in the past but that were probably still actual now, such as #1071 and #615

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR not needed

@soyuka
Copy link
Member

soyuka commented Jun 21, 2019

This is a really hot path haha, I'm not sure that it's the correct fix, maybe that #2861 is related?

@teohhanhui
Copy link
Contributor

@soyuka It's not related to #2861.

Yes, I've noticed this class before, but never understood what it's supposed to do / how it works lol...

@soyuka
Copy link
Member

soyuka commented Jun 25, 2019

It fixes metadata for classes that inherit from some other class.

@fri0z
Copy link

fri0z commented Aug 5, 2019

When can I expect this fix to be released?

@polc
Copy link
Contributor

polc commented Dec 20, 2019

Just tested this patch with my current project and it seems to fix my issues with single table inheritances :)

@teohhanhui
Copy link
Contributor

This is obsolete after #3273.

@teohhanhui teohhanhui closed this Dec 23, 2019
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

5 participants