Skip to content

Get click types from main repo#2344

Merged
ichard26 merged 4 commits into
psf:mainfrom
hukkin:patch-1
Jun 22, 2021
Merged

Get click types from main repo#2344
ichard26 merged 4 commits into
psf:mainfrom
hukkin:patch-1

Conversation

@hukkin
Copy link
Copy Markdown
Contributor

@hukkin hukkin commented Jun 22, 2021

Click types have been moved to click repo itself. See pallets/click#1856

I've had some issues with typeshed types being outdated in another project so might be good to avoid that here.

@felix-hilden felix-hilden added the ci: skip news Pull requests that don't need a changelog entry. label Jun 22, 2021
Copy link
Copy Markdown
Collaborator

@felix-hilden felix-hilden left a comment

Choose a reason for hiding this comment

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

Kiitos! I think @JelleZijlstra and others might know more about this, but I left a small comment below.


Also, the lint check fails with mypy errors, so perhaps the type hints are slightly different. If this is accepted, should that be addressed in our own code?

Comment thread .pre-commit-config.yaml Outdated
- types-toml >= 0.1.1
- types-typed-ast >= 1.4.1
- types-click >= 7.1.2
- click >= 7.1.2
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The linked Click issue was milestoned to version 8. Should this dependency be upgraded then?

@hukkin
Copy link
Copy Markdown
Contributor Author

hukkin commented Jun 22, 2021

Kiitos!

Olepas hyvä!

The linked Click issue was milestoned to version 8. Should this dependency be upgraded then?

You're right, the annotations are not present in pre v8 click distributions. I changed the requirement to v8 for type annotations.

Also, the lint check fails with mypy errors

Fixed now. This was just a drive-by PR made in web browser. I had fingers crossed everything will work with no other changes added, haha.

@JelleZijlstra
Copy link
Copy Markdown
Collaborator

We still require click>=7.1.2 in setup.py. I'm hesitant to change it there since that will require all users to also upgrade click in their environments.

Copy link
Copy Markdown
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

ship it, thanks for the pr!

@ichard26 ichard26 merged commit be16cfa into psf:main Jun 22, 2021
@hukkin hukkin deleted the patch-1 branch June 22, 2021 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: skip news Pull requests that don't need a changelog entry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants