Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix version check bug #940

Closed
wants to merge 2 commits into from
Closed

Conversation

ProGamerGov
Copy link
Contributor

By default: "1.8.0" > "1.10.0" will be equal to True, despite 1.10 being a later version that 1.8.0. This PR fixes this issue.

@NarineK NarineK requested a review from vivekmig May 12, 2022 00:30
Copy link
Contributor

@vivekmig vivekmig left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for this fix @ProGamerGov !

@facebook-github-bot
Copy link
Contributor

@vivekmig has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Aug 17, 2022
Summary:
Without the packaging library, statements like: `"1.8.0" > "1.10.0"` will be equal to True, despite v1.10 being a later version that v1.8.0.

The `packaging` library will in some cases not be already installed on a user's device, so I've also added it the `setup.py` file. It's one of the core libraries from the Python Packaging Authority, but it's not included with the base Python installation: https://packaging.python.org/en/latest/key_projects/#pypa-projects

This wasn't an issue in #940 as one the libraries in dev install has `packaging` as a dependency. So, there's no error when the tests are using the `packaging` library.

Pull Request resolved: #999

Reviewed By: vivekmig

Differential Revision: D38693600

Pulled By: NarineK

fbshipit-source-id: a5ea5ef6f2ca175d60f1638072add2fea6d31091
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants