Skip to content

Django 4.2+ compatible setup_query #167

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

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Django 4.2+ compatible setup_query #167

merged 1 commit into from
Jun 15, 2023

Conversation

bbajuzik
Copy link

This PR should fix the incompatibility described in the issue #160

@bbajuzik bbajuzik requested a review from palewire as a code owner June 12, 2023 10:06
@bbajuzik
Copy link
Author

I apologize for starting a new PR. I did notice the #166 right after I submitted this one (as usual 😅 ).

I do however believe that this PR's solution is faster, simpler, less error-prone, and more future-proof than inspecting super().setup_query and deciding which kwargs to use. And since this is an inherited method, we do not need to re-declare the defaults, we can just pass whatever the callee passed in and let the parent's implementation handle it.

@jykae
Copy link

jykae commented Jun 15, 2023

@bbajuzik would be cool if fix is this simple, probably less error prone than PR mentioned.
Anyway happy if this incompatibility would get solved for this great tool.

@palewire
Copy link
Owner

I'm happy to merge one of these patches but we need to get some testing in for 4.2 and this feature, it seems.

@palewire palewire merged commit 39d4d4c into palewire:main Jun 15, 2023
@palewire
Copy link
Owner

The fix has been released as a new version. Give it a shot and let me know if there are still any problems.

@bbajuzik bbajuzik deleted the 160-django-4.2-compatibility branch June 15, 2023 17:46
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.

4 participants