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

flake8 dependency fix for python3.7. #65

Merged
merged 2 commits into from
Oct 13, 2022
Merged

Conversation

devrimcavusoglu
Copy link
Member

Related issue PyCQA/flake8#1701

@devrimcavusoglu devrimcavusoglu added the bug Something isn't working label Oct 4, 2022
@@ -16,6 +16,7 @@ def get_requirements():
"flake8==3.9.2",
"isort==5.9.2",
"pytest>=6.2.4",
"importlib-metadata>=1.1.0,<4.3;python_version<'3.8'",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is python_version for the supported python version or has a different meaning? I am asking because we have been supporting till 3.7, 3.8 and 3.9

Copy link
Member Author

@devrimcavusoglu devrimcavusoglu Oct 5, 2022

Choose a reason for hiding this comment

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

It's a condition for managing dependencies, "it solely says if python is < 3.8, then install this.", it does not make trapper bound to python < 3.8.

On another perspective I thought for a second why not setuptools can infer this as another package?; this is a good point, I thought it was supported (for conditioning on python versions) and I got this directly from flake8, but they included this on setup.cfg rather than on requirements.txt or setup.py.

When I searched for that package on pypi, I actually founded a package python_version, so including this on setup.py instead of setup.cfg might install that redundant (?) package I think.

Update: It turns out that python_version is a reserved marker for setuptools, so there's no problem. Also, it's advised for another related issue. All good so far 🙂

@devrimcavusoglu devrimcavusoglu requested review from cemilcengiz, SecilOzkSen and Sophylax and removed request for SecilOzkSen and cemilcengiz October 5, 2022 12:53
Copy link
Member

@Sophylax Sophylax left a comment

Choose a reason for hiding this comment

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

LGTM!

@devrimcavusoglu devrimcavusoglu merged commit 4d7adaf into main Oct 13, 2022
@devrimcavusoglu devrimcavusoglu deleted the flake8-dep-fix branch October 13, 2022 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants