-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PYTHON-4451 Use Hatch as Build Backend #1644
Conversation
I filed PYTHON-4452 for the libmongocrypt failure. |
evergreen retry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. It would be good to have a little demo sometime.
@@ -136,32 +136,8 @@ def build_extension(self, ext): | |||
) | |||
ext_modules = [] | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were able to get rid of all this because of the way that requirements.txt is handled? That's awesome!
@@ -31,12 +31,10 @@ jobs: | |||
- name: Run linters | |||
run: | | |||
tox -m lint-manual | |||
- name: Check Manifest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No manifest file!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the Why Hatch? page. I'm starting to get it. Very nice.
pymongo/_version.py
Outdated
if isinstance(version_tuple[-1], str): | ||
return ".".join(map(str, version_tuple[:-1])) + version_tuple[-1] | ||
return ".".join(map(str, version_tuple)) | ||
pattern = r"(?P<major>\d+).(?P<minor>\d+).(?P<patch>\d+)(?P<rest>.*)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a function and add a few unit tests to make sure it works with a variety of versions? Eg "4.8.0.dev1", "4.8.0", "5.0", etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. We don't support "5.0", it has to have the patch version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think not supporting "5.0" is a mistake. Our 4.0 release used "4.0" for example, same for "3.0". If we don't support "5.0" then I worry we'll accidentally set the version to a 2 part at some point and lead to a bug where version_tuple
is silently set to an empty tuple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
(cherry picked from commit 2b03001)
(cherry picked from commit 2b03001)
No description provided.