-
-
Notifications
You must be signed in to change notification settings - Fork 394
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
Issue #1172 #1199
Issue #1172 #1199
Conversation
Add ExecutableMismatchViolation and remove flake8-executable
Fix checking logic so that we don't thoroughly check every comment
* Add ExecutableMismatchViolation #9 * Add WPS452 to test_noqa.py #5 * Remove dependency and related test * Add test resource files from flake8-executable * Add tests for check_valid_shebang #7 * Implement the full checking logic (#13) * Fix linting problem (#14) * Clean up code (#15) Fix checking logic so that we don't thoroughly check every comment * Fix permissions (#16) * Ignore failing test files * Update test file to be compatible with the new visitor * Fix linter issues * Make a new visitor to handle files with 0 comments (#17) * Exclude test files from linting Co-authored-by: Hanzhang <[email protected]> Co-authored-by: jrutqvist <[email protected]> Co-authored-by: Gabriel Chang <[email protected]>
Pull Request Test Coverage Report for Build 2758
💛 - 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.
Thanks! Looking good! 🥳
We need to fix some testing related things and minor code issues and we are good to go!
tests/fixtures/test_executable_mismatch_resources/exe001_neg.py
Outdated
Show resolved
Hide resolved
Use temporary files for testing
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.
Great job! Thank you!
There are a lot of comments fixed. And we are moving towards an awesome new rule.
Keep up the good work! 👍
tests/test_visitors/test_tokenize/test_comments/test_valid_shebang.py
Outdated
Show resolved
Hide resolved
tests/test_visitors/test_tokenize/test_comments/test_valid_shebang.py
Outdated
Show resolved
Hide resolved
tests/test_visitors/test_tokenize/test_comments/test_valid_shebang.py
Outdated
Show resolved
Hide resolved
bcc8275
to
d9a3c5b
Compare
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.
Awesome. This was the last round of review.
Just two cosmetic changes and we are ready to go!
Thank you very much for working on this! This is an important feature.
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.
Thanks a lot! Awesome job, I am going to merge this into 0.15
🎉
Oh, I am merging this into @adambenali can you please rebase this PR or allow changes from maintainers? |
@sobolevn, edits from maintainers are already allowed. |
Oh sorry, It should work now, I've removed the protection from the branch ! |
I have made things!
Checklist
CHANGELOG.md
Related issues
We first wanted to use the existing
FileMagicCommentsVisitor
, because it would make more sense as a shebang is also a magic comment. This wouldn't work however since we also need to check cases where there is no comment in the file. So we created a new visitor for this.The behavior is exactly the same as
flake8-executable
.🙏 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.