Restrict the maximum number of base classes aka mixins #264 #278
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 538
💛 - Coveralls |
sobolevn
requested changes
Oct 19, 2018
| from wemake_python_styleguide.visitors.ast.classes import WrongClassVisitor | ||
|
|
||
| correct_count = """ | ||
| class CorrectClassName(FirstParentClass, |
There was a problem hiding this comment.
Please, remove the tabulation.
There was a problem hiding this comment.
We also use different type of (...).
It should be like so:
class Some(
Parent1,
Parent2,
):|
|
||
|
|
||
| too_many_count = """ | ||
| class SomeClassName(FirstParentClass, |
There was a problem hiding this comment.
The same about tabulation and ()
| @@ -181,6 +181,12 @@ class Configuration(object): | |||
| 'Maximum number of conditions in a `if` or `while` node.', | |||
| ), | |||
|
|
|||
| _Option( | |||
| '--max-classes-number', | |||
There was a problem hiding this comment.
It should be --max-base-classes
| @@ -181,6 +181,12 @@ class Configuration(object): | |||
| 'Maximum number of conditions in a `if` or `while` node.', | |||
| ), | |||
|
|
|||
| _Option( | |||
| '--max-classes-number', | |||
| defaults.MAX_CLASSES_NUMBER, | |||
| Reasoning: | ||
| It is almost never possible to navigate | ||
| to the desired method of a parent class | ||
| when you need it with multiple mixin. |
| It is almost never possible to navigate | ||
| to the desired method of a parent class | ||
| when you need it with multiple mixin. | ||
| It is hard to understand mro and super calls. |
There was a problem hiding this comment.
mro and super should be wrapped with double ``
| class SomeClassName(FirstParentClass, SecondParentClass): ... | ||
|
|
||
| # Wrong: | ||
| class SomeClassName(FirstParentClass, |
There was a problem hiding this comment.
The same about () applies here
sobolevn
requested changes
Oct 20, 2018
| class CorrectClassName( | ||
| FirstParentClass, | ||
| SecondParentClass, | ||
| ThirdParentClass |
| SecondParentClass, | ||
| ThirdParentClass, | ||
| CustomClass, | ||
| AddedClass |
| when you need it with multiple mixin. | ||
| It is hard to understand mro and super calls. | ||
| when you need it with multiple mixins. | ||
| It is hard to understand 'mro' and 'super' calls. |
There was a problem hiding this comment.
Please, use double `` to wrap mro and `super`. Like here:
sobolevn
approved these changes
Oct 20, 2018
|
@sobolevn Used it personally. I am new in python, and this is good opportunity to learn it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have made things!
Checklist
CHANGELOG.mdRelated issues
Closes #264