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

Fixed typing #1408

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Fixed typing #1408

merged 1 commit into from
Jun 4, 2021

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Jun 2, 2021

Changelog-friendly one-liner: Fixed typing

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

pip 21.0 has internal changes that break at least our typing and we will need significant changes to support 21.x for typing. For the moment we ceil pip version for mypy and assure running mypy does get us consistentresults inside and outside pre-commit.

Fixes type change introduced by click 8

A previous version of this patch attempted to pin down pip version in setup.cfg but I soon found that piplatest tox env
started to fail as in-tree-build feature was unknown to it. I suppose we will have to bump minimal pip version required
to >=21 as soon we upgrade type hints .

@ssbarnea ssbarnea added this to the 6.1.1 milestone Jun 2, 2021
@ssbarnea ssbarnea added the ci Related to continuous integration tasks label Jun 2, 2021
@ssbarnea ssbarnea marked this pull request as ready for review June 2, 2021 09:12
@ssbarnea ssbarnea added maintenance Related to maintenance processes pip Related to pip labels Jun 2, 2021
@ssbarnea ssbarnea requested a review from webknjaz June 2, 2021 09:13
@codecov

This comment has been minimized.

setup.cfg Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@ssbarnea
Copy link
Member Author

ssbarnea commented Jun 2, 2021

I think we should ignore the coverage issue as covering for both versions of click 7 and 8 would make our CI pipelines too complex. It is just one line not covered. It would be much easier to just bump minimal version of click required than covering for both versions.

@ssbarnea
Copy link
Member Author

ssbarnea commented Jun 3, 2021

It is needed to pass mypy because the type changed in click 8 vs 7.

pip 21.0 has internal changes that break at least our typing and we
will need signifiant changes to support 21.x. For the moment we
ceil pip version and assure running mypy does get us consistent
results inside and outside pre-commit.

Fixes type change introduced by click 8
@webknjaz webknjaz merged commit 16ff084 into jazzband:master Jun 4, 2021
@ssbarnea ssbarnea deleted the fix/mypy branch June 5, 2021 07:05
@atugushev atugushev modified the milestones: 6.1.1, 6.2.0 Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to continuous integration tasks maintenance Related to maintenance processes pip Related to pip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants