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.

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

Update pep8-naming compatibility with flake8 #210

Merged
merged 9 commits into from Dec 16, 2022

Conversation

stdedos
Copy link
Contributor

@stdedos stdedos commented Dec 14, 2022

pep8-naming tests fail for flake8 v3/v4; avoid suggesting that it works

Additionally, v6 added one more required kwarg. Let's add that one too.

Signed-off-by: Stavros Ntentos 11300730-stavros-relex@users.noreply.gitlab.com

requirements.txt Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@stdedos
Copy link
Contributor Author

stdedos commented Dec 15, 2022

Let me know what you think of the changes!

I resolved the discussions that I assumed the decision & action was clear; please resolve/elaborate on the tox.ini discussion

Copy link
Member

@jparise jparise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appear to be text failures related to the new tox.ini dependency matrix. I'm not able to look more closely at the moment, so would you be able to give those a look @stdedos?

If you'd like to just bump the install_requires dependency as its own change, I'd be glad to approve that separately.

tox.ini Outdated Show resolved Hide resolved
@stdedos
Copy link
Contributor Author

stdedos commented Dec 16, 2022

If you'd like to just bump the install_requires dependency as its own change, I'd be glad to approve that separately.

If "proving actually" that v3/v4 is not supported while doing the change itself, I could do it. Although, I'd guesstimate it will be equivalent effort to do either.

@stdedos
Copy link
Contributor Author

stdedos commented Dec 16, 2022

I'm not able to look more closely at the moment, so would you be able to give those a look @stdedos?

Oh yeah! Here's my v6 problem: https://github.com/PyCQA/pep8-naming/actions/runs/3702319329/jobs/6289277028 😛

Stavros Ntentos and others added 3 commits December 16, 2022 15:41
`pep8-naming` tests fail for `flake8` v3/v4; avoid suggesting that it works.

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
* No `requirements.txt` file
* No `install_requires` upper limit on `flake8`

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
* `py37` has no `flake8 v6`
* `flake8.options.manager.OptionManager` seems to require a `formatter_names` `kwarg`
  PyCQA/flake8@48b2919 etc
  seem to indicate that `[]` is what we need, "since we are not a formatter library"
  (Documentation https://flake8.pycqa.org/en/latest/internal/option_handling.html seems non-existent)

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
@stdedos stdedos force-pushed the test/flake8-fix-compatibility branch from 086f02f to 154683d Compare December 16, 2022 13:43
@stdedos
Copy link
Contributor Author

stdedos commented Dec 16, 2022

Someone pls workflows 🙏

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
run_tests.py Outdated Show resolved Hide resolved
Additionally, define `extras_require:dev` with `tox` as an extra dependency.

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
@stdedos
Copy link
Contributor Author

stdedos commented Dec 16, 2022

I assumed that requirements-dev.txt is also not wanted; hence I specified extras_require:dev 🙏

setup.py Outdated Show resolved Hide resolved
Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
@stdedos
Copy link
Contributor Author

stdedos commented Dec 16, 2022

Finally CI likes my changes 🎉 🚀

Additionally, rename `flake8#` to `flake8-v#`

Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
tox.ini Outdated Show resolved Hide resolved
Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com>
@jparise jparise merged commit af89472 into PyCQA:main Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants