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

Move __version__ single source of truth to package __init__ #36

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

Erotemic
Copy link
Contributor

@Erotemic Erotemic commented Dec 3, 2023

Currently the main Python module don't have a __version__ property. This is a pseudo-standard property that many packages do include, and its helpful to have. I find putting __version__ in the top-level __init__.py is a good place for the single-source-of-truth for the version number. I've included code in the setup.py to parse that information from __init__.py file, so when versions change, you only need to bump the version in one place.

@Erotemic Erotemic mentioned this pull request Dec 3, 2023
@oddstr13
Copy link
Member

This is a lot of code for something rather simple – I tend to handle this with a simple regex in my own projects

https://github.com/oddstr13/jellyfin-plugin-repository-manager/blob/493e3aec37da39f8faa7a2ee308109edebe856b4/setup.py#L6-L7

with open('jprm/__init__.py', 'r') as fh:
    version = re.search(r'__version__ *= *["\'](.*?)["\']', fh.read()).group(1)

In jprm I also have CI to automate the version change as part of the release prep, but I see that this repo doesn't have anything of that sort yet. https://github.com/oddstr13/jellyfin-plugin-repository-manager/blob/493e3aec37da39f8faa7a2ee308109edebe856b4/.github/workflows/release-next-create-pr.yaml#L61-L69

@Erotemic
Copy link
Contributor Author

Whatever the maintainers feel comfortable with is fine. The main thing I care about is that __version__ is defined.

This code is generated by my xcookie repo templater. While it is longer than a regex, this code correctly handles edge cases that the regex doesn't. I like things that treat code as code, but that's my opinion.

@iwalton3
Copy link
Member

I usually just throw the version in the natural places and use a script to yell at me if I forget to change them all.

https://github.com/jellyfin/jellyfin-mpv-shim/blob/9d2d2b83e84b297c36a1d35634569a0269c485a7/gen_pkg.sh#L67-L87

@iwalton3
Copy link
Member

I'd accept this PR if it just throws the version into the two places. I could add something like gen_pkg.sh myself.

@Erotemic
Copy link
Contributor Author

Erotemic commented Jan 21, 2024

I think it's important to have a single-source-of-truth for information like the version. It looks like the new pyproject.toml configuration has built in support for this sort of versioning. I think a better solution would be to switch to that.

@Erotemic
Copy link
Contributor Author

I've added a commit that makes this change. This removes the problem of there being a lot of code in the setup.py. In fact there is no more code in the setup.py, the entire build process is now declarative. I build a wheel before / after this change to ensure that all metadata in the .whl/METADATA is preserved. (Note: author name is now included in the author email field using a git-like name <email> format).

@iwalton3
Copy link
Member

Sure, this looks fine.

@iwalton3 iwalton3 merged commit 913322c into jellyfin:master Jan 21, 2024
5 checks passed
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