-
-
Notifications
You must be signed in to change notification settings - Fork 384
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
[#271] Restricted variable names with more than one consecutive underscore #277
Conversation
@sobolevn I'm really not sure that I've made some right things. First is moving |
Pull Request Test Coverage Report for Build 532
💛 - Coveralls |
Pull Request Test Coverage Report for Build 573
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the pattern that you have found in this file to split in half.
But, I dislike the way you have named new files.
Module name that contains helpers
is an anti-pattern.
And this is not a joke, we actually have a rule to check it: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/constants.py#L128
So, I suggest to rename name_checks
into naming
and name_helpers
into name_nodes
.
How does that sound to you?
Btw, thanks for the awesome job you have done to implement this feature.
@@ -0,0 +1,27 @@ | |||
# -*- coding: utf-8 -*- | |||
import ast |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing empty line after # -*-
# Correct: | ||
some_value = 5 | ||
__magic__ = 5 | ||
# Wrong: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be an empty line here
This naming rule already exist for module names. | ||
|
||
Example:: | ||
# Correct: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be an empty line here before # Correct:
__magic__ = 5 | ||
# Wrong: | ||
some__value = 5 | ||
Note: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be an empty line here before Note:
@Roxe322 considering your thoughts about |
Hi, @sobolevn , I've fixed missing blank lines and renamed modules as you suggested. What should I do with DRY problem in tests? Also, I've just noticed that Travis broke in 3.7 tests. Why such problem occured and why in 3.6 everything is ok? I'm using 3.7 on my local machine and pytest working normally. |
You have the same problem as #280 I am not sure what happens here. |
@Roxe322 can you please |
@sobolevn Ok, also i'll fix merge conflicts |
@Roxe322 please, check your build status. |
@sobolevn Yes, I'm working on this, just not very successful merge conflicts resolving |
Awesome! Thanks! |
I have made things!
Checklist
CHANGELOG.md
Related issues