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

deps: support flake8 v5 #116

Merged
merged 3 commits into from Aug 4, 2022
Merged

Conversation

nhymxu
Copy link
Contributor

@nhymxu nhymxu commented Aug 1, 2022

Close #115

@nhymxu
Copy link
Contributor Author

nhymxu commented Aug 1, 2022

Test case on my PR run successfully: nhymxu#1

dbkeitel added a commit to PyFstat/PyFstat that referenced this pull request Aug 1, 2022
- try to work around isort incompatibility
  while waiting for gforcada/flake8-isort#116
dargor added a commit to dargor/dargor_gentoo_overlay that referenced this pull request Aug 2, 2022
See gforcada/flake8-isort#116

Signed-off-by: Gabriel Linder <linder.gabriel@gmail.com>
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]

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

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
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?

flake8_isort.py Outdated
@@ -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

@@ -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.

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

@gforcada
Copy link
Owner

gforcada commented Aug 4, 2022

Thanks all, specially @nhymxu for doing the PR and all the feedback from the others, I would like a few changes being made and then I can cut a release 👍🏾

@nhymxu
Copy link
Contributor Author

nhymxu commented Aug 4, 2022

My PR run successfully all test case.
nhymxu#2

@gforcada gforcada merged commit a31c6e3 into gforcada:master Aug 4, 2022
@gforcada
Copy link
Owner

gforcada commented Aug 4, 2022

flake8-isort 4.2.0 is out! 🎉

Please take a minute to try it and report if it works as expected 😄

Thanks for the code contributions and the comments! 💯

@rydenius
Copy link

rydenius commented Aug 4, 2022

Works for me. Thanks all.

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.

Incompatible with flake8 5.0.0 ?
5 participants