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

Fix __version__ and get rid of __version_info__ #65

Merged
merged 2 commits into from
Dec 8, 2021
Merged

Conversation

JOJ0
Copy link
Contributor

@JOJ0 JOJ0 commented Dec 5, 2021

Fixing __version__ shouldn't hurt, removing __version_info__ entirely is a proposal. Tell me what you think @AnssiAhola and @alifhughes.
Some details in the commit message.

- Since both of those vars were wrong for ages, it shoudn't do any harm.
- __version_info__ is ment to be used to programatically compare
  versions as described here: https://softwareengineering.stackexchange.com/questions/375922/whats-the-version-info-function-in-a-pypi-package
- An alternative to removing it would be to implement a oneliner
  creating the 3-digit tuple as described in above link.
- Starting with Python 3.8. there are better and recommended ways to
  handle package versioning: https://docs.python.org/3/library/importlib.metadata.html#distribution-versions
@AnssiAhola
Copy link
Contributor

I'd lean more towards the oneliner for version_info, incase someone wants to use it, although i dont see much use for it 😅

@JOJ0
Copy link
Contributor Author

JOJ0 commented Dec 6, 2021

Hi Anssi, thanks for giving feedback so quickly! OK, is fine with me as well. Yeah, why not, actually it's a simple oneline-comprehension thing with nothing special/version-dependent in it. I will add it. I'll wait a day or so to give @alifhughes a chance to have a quick look.

- Add in __version_info__ again and use a generator expression to create
  the tuple.
- Will work with our regular versioning (1.1.1),
- but won't if non-digit characters are used (1.1.1-rc1)
- The latter shouldn't be a problem since we don't usually do that.
@alifhughes
Copy link
Contributor

Hey guys, sorry for the delay - looks good to me!

I'd lean more towards the oneliner for version_info, incase someone wants to use it, although i dont see much use for it 😅

I'd agree, best to keep these things even though I don't see too much of a use of it myself.

@JOJ0 JOJ0 merged commit 80eae25 into joalla:master Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants