Remove arguments from InheritanceQuerySetMixin._clone()#567
Remove arguments from InheritanceQuerySetMixin._clone()#567foarsitter merged 1 commit intojazzband:masterfrom
InheritanceQuerySetMixin._clone()#567Conversation
The `QuerySet._clone()` method has no arguments in either Django 3.2 or 4.1, so I'm assuming the arguments were necessary for a version of Django that is no longer supported by `django-model-utils`.
Codecov Report
@@ Coverage Diff @@
## master #567 +/- ##
=======================================
Coverage 95.12% 95.12%
=======================================
Files 6 6
Lines 820 820
=======================================
Hits 780 780
Misses 40 40
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
This PR will break backwards compatibility right? Is it an idea to just accept |
It doesn't break any existing functionality, but instead of any passed arguments being silently ignored, a However, as
Yes, but if we want to be that careful, probably it should catch both I'll leave the decision up to you: personally, I don't think it's likely this change will break any existing code, but if you prefer to be extra cautious, I'll make it accept |
|
Extra cautiousness isn't necessary since your argument about the private nature of the method is a good one. |
Problem
InheritanceQuerySetMixin._clone()accepts and then ignores aklassandsetupargument, plus any keyword arguments. TheQuerySet._clone()method has no arguments in either Django 3.2 or 4.1 and when using a static code checker likemypy, this is reported as incompatible base classes forInheritanceManager.Solution
I'm assuming the arguments were necessary for a version of Django that is no longer supported by
django-model-utils. Therefore I removed the extra arguments.Commandments
_clone()method was already covered.)CHANGES.rstfile to describe the changes, and quote according issue withGH-<issue_number>.I chose to not add an entry in
CHANGES.rst, as this is a cleanup that should not be noticeable to users.