Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove flake8-executable dependency #1172

Closed
sobolevn opened this issue Feb 22, 2020 · 6 comments · Fixed by #1199
Closed

Remove flake8-executable dependency #1172

sobolevn opened this issue Feb 22, 2020 · 6 comments · Fixed by #1199
Assignees
@adambenali
Labels
dependencies Pull requests that update a dependency file feature New feature or request help wanted Extra attention is needed level:starter Good for newcomers pr-merged
Milestone
Version 0.14 aka ...

Comments

@sobolevn
Copy link
Member

We need to vendor this dependency to our own codebase:

  • create new violation
  • create new logical check
  • write tests

Implementation: https://github.com/xuhdev/flake8-executable/blob/abf67771703b2f06f02a3fedb1dd97299583a4c3/flake8_executable/__init__.py#L92-L128

Related: #1139
@sobolevn sobolevn added feature New feature or request help wanted Extra attention is needed level:starter Good for newcomers dependencies Pull requests that update a dependency file labels Feb 22, 2020
@sobolevn sobolevn added this to the Version 0.14 aka python3.8 milestone Feb 22, 2020
@sobolevn sobolevn mentioned this issue Feb 26, 2020
Closed
@sobolevn
Copy link
Member Author

But, we only need a single violation for this. Let's call it ExecutableMismatchViolation and change its error message instead. Not the code.

@adambenali
Copy link
Contributor

I'd like to work on it !

@sobolevn
Copy link
Member Author

🤝 thanks a lot!

@sobolevn
Copy link
Member Author

@pandaseal does this plugin work on Windows without building?

@pandaseal
Copy link
Contributor

pandaseal commented Feb 26, 2020 •
edited

Yes the plugin works.

  • I works in Windows natively (in cmd and Powershell)
  • It works in the home dir of the WSL
  • It does not work in the windows partion of the WSL (but this is not due to the plugin but the overwriting of permissions problem)

However, the reason the build does not work on windows is the UnicodeDecodeError referenced in issue #337 and tests failing

@sobolevn
Copy link
Member Author

sobolevn commented Feb 26, 2020 •
edited

@pandaseal you would do us a big (like 2 years big) favour if you tackle this windows build issue down. I don't own a windows machine and too lazy to setup and an emulation, so this is an actual problem for us that is not solved for 2 years now.

We can even bring back the windows CI with https://docs.travis-ci.com/user/reference/windows/

If you are willing to work on this, we can reopen #337

@ChangGabriel ChangGabriel mentioned this issue Feb 27, 2020
Closed
adambenali added a commit to adambenali/wemake-python-styleguide that referenced this issue Feb 28, 2020
@adambenali
Merge master into issue-wemake-services#1172
818d30c
@adambenali adambenali mentioned this issue Feb 28, 2020
Merged
4 tasks
@helpr helpr bot added the pr-available label Feb 28, 2020
adambenali added a commit to adambenali/wemake-python-styleguide that referenced this issue Feb 29, 2020
@adambenali
Merge master into issue-wemake-services#1172
e40c449
adambenali added a commit to adambenali/wemake-python-styleguide that referenced this issue Feb 29, 2020
@adambenali
Merge branch 'master' into issue-wemake-services#1172
df311a8
sobolevn added a commit to adambenali/wemake-python-styleguide that referenced this issue Mar 6, 2020
@sobolevn
Merge branch 'master' into issue-wemake-services#1172
633e2f6
sobolevn added a commit that referenced this issue Mar 6, 2020
@adambenali @hanzhsun
@ChangGabriel @jrutqvist @sobolevn
Issue #1172 (#1199)
cf3aef9 
* Add ExecutableMismatchViolation #9

* Add logic for ExecutableMismatch

* Add WPS452 to test_noqa.py #5

* Remove dependency and related test

* 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)

* Make a new visitor to handle files with 0 comments (#17)

* Add tests for ExecutableMismatchViolation (#18)

* 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 <48179160+hanzhsun@users.noreply.github.com>
Co-authored-by: jrutqvist <joar.rutqvist@gmail.com>
Co-authored-by: Gabriel Chang <Gc.chang95@gmail.com>

* Update CHANGELOG.md

* Moved test resources to fixtures to be ignored by autopep8

* Add integration test

* Refactor tests

* Make the requested changes
Use temporary files for testing

* Update noqa.py

* Make additional requested changes

* Fix small formatting and typing issues

* Update comments.py

* Update comments.py

Co-authored-by: hanzhsu <emily_1999@link.cuhk.edu.hk>
Co-authored-by: Gabriel Chang <Gc.chang95@gmail.com>
Co-authored-by: Hanzhang <48179160+hanzhsun@users.noreply.github.com>
Co-authored-by: jrutqvist <joar.rutqvist@gmail.com>
Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@helpr helpr bot added pr-merged and removed pr-available labels Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adambenali adambenali

Labels
dependencies Pull requests that update a dependency file feature New feature or request help wanted Extra attention is needed level:starter Good for newcomers pr-merged
Projects
None yet
Milestone
Version 0.14 aka python3.8
Development

Successfully merging a pull request may close this issue.

Issue #1172 adambenali/wemake-python-styleguide
3 participants
@sobolevn @pandaseal @adambenali