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.

Sign up for GitHub

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

Jump to bottom

Change plugin hook condition for analyzing model base class #1668

Merged
merged 1 commit into from
Aug 31, 2023
Merged

Change plugin hook condition for analyzing model base class #1668

merged 1 commit into from
Aug 31, 2023

Conversation

flaeppe
Copy link
Member

@flaeppe flaeppe commented Aug 31, 2023

Should simplify the hook requirement and remove some unnecessary code.

Related issues

Refs: #1663 (comment)

@flaeppe
Change plugin hook condition for analyzing model base class
65370c9 
Should simplify the hook requirement and remove some unnecessary code.
@flaeppe flaeppe requested a review from sobolevn August 31, 2023 08:51
@flaeppe flaeppe mentioned this pull request Aug 31, 2023
Merged
sobolevn
sobolevn approved these changes Aug 31, 2023
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! It worked 🎉

@flaeppe flaeppe merged commit 81a543a into typeddjango:master Aug 31, 2023
@flaeppe flaeppe deleted the fix/model-hook-condition branch August 31, 2023 09:38
@sobolevn
Copy link
Member

One idea: what about models with ModelBase subclasses used as metaclasses?
Do we care to support them?

@flaeppe
Copy link
Member Author

flaeppe commented Aug 31, 2023

One idea: what about models with ModelBase subclasses used as metaclasses? Do we care to support them?

Hm, interesting. Perhaps we should wait for that case to begin with?

I suppose we have to iterate the metaclass_type.info.mro too in that case.

(
    metaclass_type.type.fullname == fullnames.MODEL_METACLASS_FULLNAME
    or metaclass_type.type.has_base(fullnames.MODEL_METACLASS_FULLNAME)
)

?

Though hard to say if we can or cannot support those..

@sobolevn
Copy link
Member

I think that it is pretty easy to support, so why not?

@flaeppe flaeppe mentioned this pull request Aug 31, 2023
Closed
@flaeppe flaeppe mentioned this pull request Dec 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobolevn sobolevn sobolevn approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@flaeppe @sobolevn