fix: add a check to ensure the MRO is correct #1352
Conversation
|
Thanks for the MR. Could you elaborate why this matters? I know what you can override methods based on the order, but other than that, I'm unsure. Would be interesting to know @JohnVillalovos |
Sure. Let me write up a better explanation and post it. It will have to be after my work-day is over here in Oregon. |
I have put comments in the test file that explain it better. This PR is assuming that #1344 will be merged. As that is what triggered this issue. |
Codecov Report
@@ Coverage Diff @@
## master #1352 +/- ##
=======================================
Coverage 80.21% 80.21%
=======================================
Files 73 73
Lines 3801 3801
=======================================
Hits 3049 3049
Misses 752 752
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@max-wittig Hopefully I have answered your question. |
|
Any more questions or comments about this? |
|
@nejch Thanks for the feedback. I'll try to get some time to work on this in the next few days. |
|
@nejch Updated. I removed the debug code and moved the comments into the docstring. |
Add a check to ensure the MRO (Method Resolution Order) is correct for classes in
gitlab.v4.objects when doing type-checking.
An example of an incorrect definition:
class ProjectPipeline(RESTObject, RefreshMixin, ObjectDeleteMixin):
^^^^^^^^^^ This should be at the end.
Correct way would be:
class ProjectPipeline(RefreshMixin, ObjectDeleteMixin, RESTObject):
Correctly at the end ^^^^^^^^^^
Also fix classes which have the issue.
This should be merged after:DONE#1344
Add a check to ensure the MRO (Method Resolution Order) is correct for classes in
gitlab.v4.objects.
Also fix classes which have the issue.