Pin flake8 - #1781
Conversation
7.2.0 introduces a new rule on unused globals/nonlocals (F824) which will be addressed separately.
Address the F824 rule violation and update the pin to major version 7.
| "cram >=0.7", | ||
| "deepdiff >=4.3.2, <8.0.0", | ||
| "flake8", | ||
| "flake8 >=7.0.0, <8", |
There was a problem hiding this comment.
The error which prompted this PR was due to 7.1.2 to 7.2.0 bump, so wouldn't this pin not have prevented the CI failure and thus keep the door open to future similar issues?
There was a problem hiding this comment.
Yes, but I think it's still useful to have new flake8 versions. Note that Nextstrain CLI has no upper pin on most dev deps, including flake8. But maybe, related to #1557, we should pin exact and update ~regularly?
There was a problem hiding this comment.
I agree with the desire to keep current for deps such as these. But having CI fail like it did in #1756 (comment) isn't good for development and should be avoided. A better approach would be a dependebot-like PR for flake8 upgrades.
There was a problem hiding this comment.
This is well-suited for #1538. I'll try to revisit that PR sometime.
3 participants
Description of proposed changes
Flake8 was previously unpinned which led to a linting error in the latest 7.2.0 release, as noted in #1756 (comment). This PR initially pins to <7.2.0, then since there is only one linting error, I went ahead and fixed it, relaxing the pin. It should still be beneficial to use new flake8 versions. We can always tighten the pin to not install a specific version if new errors become annoying.
Checklist
Check if you need to add a changelog messagedev changeCheck if you need to add testsCheck if you need to update docs