Drop python 3.6 support #529
Conversation
sanders41
added
the
breaking-change
|
@alallema if you want to merge this |
There was a problem hiding this comment.
Thanks, @sanders41 for this PR ❤️ !!!
I just had few questions on it
| @@ -1,7 +1,10 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
Sorry for this question but why did we import __future__ here if we did use it on this file?
There was a problem hiding this comment.
Technically it isn't needed but it doesn't hurt anything to have it. I added it because I have had cases where I didn't put it, then later went back and added something that need it and forgot to import __future__. The issue there is python 3.10+ works without it so everything seems good, but then errors show up if you try to use < 3.10.
So it's just there as a future __future__ 😄 safety net. If you would rather remove I can.
| @@ -1,3 +1,5 @@ | |||
| from __future__ import annotations | |||
| from typing import Any, List, Dict, Optional | ||
| from __future__ import annotations | ||
|
|
||
| from typing import Any, Dict, Optional |
There was a problem hiding this comment.
Why we didn't remove Dict too in this file?
There was a problem hiding this comment.
Pydantic, which is what CamelBase is using behind the scenes, doesn't like __future__ style annotations. Doing it this way makes Pydantic happy while still providing the benefits it brings. To the end user it is all the same, they don't see any difference with the different annotation types.
There was a problem hiding this comment.
Thanks @sanders41 for this PR and for your explanations ❤️ 🚀
LGTM! 🔥 🔥
|
bors merge |
|
Configuration problem: |
@sanders41 - Oopsy, bors is right we need to modify the |
|
Oops, I fixed the toml file. |
|
bors merge |
|
Build succeeded:
|
Pull Request
This drops support for Python 3.6 and updates to 3.7+ syntax.
What does this PR do?
Fixes #378
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!