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

deps: support flake8 v5 #116

Merged
merged 3 commits into from Aug 4, 2022
Merged

deps: support flake8 v5 #116

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
267a559
deps: support flake8 v5
nhymxu Aug 1, 2022
3f3615a
revert package version as-is
nhymxu Aug 4, 2022
31cccb3
bump flake8 version on test matrix
nhymxu Aug 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
View file
Open in desktop
Expand Up @@ -19,8 +19,8 @@ jobs:
strategy:
matrix:
python-version: ['3.6', '3.7', '3.8', '3.9', '3.10', 'pypy3']
isort: [4.3.21, 5.9.3]
flake8: [3.9.2, 4.0.1]
isort: [4.3.21, 5.10.1]
flake8: [3.9.2, 4.0.1, 5.0.1]
Copy link

Choose a reason for hiding this comment

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

flake8 has bumped to 5.0.4, can you update this target and test again, please?

Copy link
Owner

Choose a reason for hiding this comment

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

@nhymxu would you mind applying this suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GriceTurrble @gforcada

I bump version to 5.0.4


steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion flake8_isort.py
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@
import warnings


__version__ = '4.1.2.post1.dev0'
__version__ = '4.2.0'
Copy link
Owner

Choose a reason for hiding this comment

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

Please leave this as it is, when a new release is cut it is automatically handled by zest.releaser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gforcada I revert it



class Flake8IsortBase(object):
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Expand Up @@ -55,7 +55,7 @@ def get_version(file="flake8_isort.py"):
include_package_data=True,
zip_safe=False,
install_requires=[
'flake8 >= 3.2.1, <5',
'flake8 >= 3.2.1, <6',
Copy link
Owner

Choose a reason for hiding this comment

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

maybe we can drop the upper limit? I would say yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, as my view, I think we should keep it.
We don't know about what happend when flake8 bump to version 6.
So I think we need verify again when major change is better.

But if you think remove is okay. I will help remove it.

Copy link

Choose a reason for hiding this comment

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

It's a good checkpoint, as long as updates come fast enough to account for it.

Copy link

Choose a reason for hiding this comment

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

I would recommend removing the upper bound, you're just going to have this same scramble every time I release a major version and I'm going to have to deal with the grumpy users every time that report stuff to the wrong place

Copy link

Choose a reason for hiding this comment

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

The problem seems to be that 3.0.0 of flake8-isort was released without upper bound, so when a new major release of flake8 comes along, that one is dug up by dependency resolution and all bug fixes between 3.0.0 and now are resurfaced. I agree an upper bound is a good checkpoint if it prevents dependencies from being satisfied until the new version is confirmed to be compatible. But when a 2+ year old release surface it is pretty confusing.

Copy link

Choose a reason for hiding this comment

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

generally I find the best approach for libraries is to only include < when there is a known broken version and even then generally prefer != with the assumption it will be fixed

'isort >= 4.3.5, <6',
],
extras_require={
Expand Down