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

Please be mindful of the versions #161

Open
penguinpee opened this issue Nov 4, 2023 · 10 comments
Open

Please be mindful of the versions #161

penguinpee opened this issue Nov 4, 2023 · 10 comments

Comments

@penguinpee
Copy link
Contributor

I encountered several issues with the version of the package.

In tag: 0.8.1:

__version__ = "0.8"

In tag 0.8:

__version__ = "0.8b2"

In tag 0.7.3:

version='0.7.2',

Either make this a release step and test that it is set correctly or use tooling that automatically adjusts the version according to the latest tag.

Right now, this would lead to issues when upgrading the package in Fedora. For example, I just built 0.8 and that has:

$ rpm -q --provides -p results_python-adjustText/0.8/1.fc40/python3-adjustText-0.8-1.fc40.noarch.rpm 
python-adjustText = 0.8-1.fc40
python3-adjustText = 0.8-1.fc40
python3.12-adjustText = 0.8-1.fc40
python3.12dist(adjusttext) = 0.8~b2
python3dist(adjusttext) = 0.8~b2

The last two lines are taken from whatever the wheel provides. They should be corresponding to the actual version.

@penguinpee
Copy link
Contributor Author

There's more trouble.

Since 0.8.1 is a tag as well as a branch, the following URL is ambiguous: https://github.com/Phlya/adjustText/archive/0.8.1/adjustText-0.8.1.tar.gz

Downloading that tarball, as our tooling does, results in a tarball that is not a tarball, but a text file with the following line in it:

the given path has multiple possibilities: #<Git::Ref:0x00007fa47b7e35c0>, #<Git::Ref:0x00007fa47b7e28a0>

To avoid this in the future, either prefix the tags with a v or use a different branch name, e.g. 0_8_1 or release_0.8.1.

@musicinmybrain
Copy link

There's more trouble.

Since 0.8.1 is a tag as well as a branch, the following URL is ambiguous: https://github.com/Phlya/adjustText/archive/0.8.1/adjustText-0.8.1.tar.gz

Downloading that tarball, as our tooling does, results in a tarball that is not a tarball, but a text file with the following line in it:

the given path has multiple possibilities: #<Git::Ref:0x00007fa47b7e35c0>, #<Git::Ref:0x00007fa47b7e28a0>

To avoid this in the future, either prefix the tags with a v or use a different branch name, e.g. 0_8_1 or release_0.8.1.

Note that it’s possible to work around this by writing the URL as https://github.com/Phlya/adjustText/archive/refs/tags/0.8.1/adjustText-0.8.1.tar.gz instead, although this is not ideal.

@penguinpee
Copy link
Contributor Author

Note that it’s possible to work around this by writing the URL as https://github.com/Phlya/adjustText/archive/refs/tags/0.8.1/adjustText-0.8.1.tar.gz instead, although this is not ideal.

Indeed. Another option for Fedora packagers fond of forge macros is to use the commit hash for the tag. Something like:

Version:        0.8.1
Release:        %autorelease -n
Summary:        Automatic label placement for matplotlib
BuildArch:      noarch
# Use commit because tag is ambiguous with branch using the same name
%global commit e3904a168581d1a9f67e7399ac01f6ad6f578c43
%forgemeta
License:        MIT
URL:            %forgeurl
Source0:        %forgesource

@Phlya
Copy link
Owner

Phlya commented Jan 6, 2024

Thank you all for the comments. I just released a new version (1.0!), did I do it correctly?

@penguinpee
Copy link
Contributor Author

I just released a new version (1.0!), did I do it correctly?

Well, it's a match (kind of). Looking in _version.py, the version is 1.0. GitHub has it as 1.0.0. Normalized, these are referring to the same version. However, it might be even cleaner if you'd use semantic versioning in _version.py as well, e.g. define the version as x.y.z.
But why did you switch to munged versions with regards to the tags. The tag for the 1.0(.0) release is v1.0, while all previous tags just used the version without the v prefix. It's not a big deal and a lot of projects do that. I was just wondering why you started with it now. Same here, you might wanna use semantic versioning for tags as well.

@Phlya
Copy link
Owner

Phlya commented Jan 8, 2024

the version is 1.0. GitHub has it as 1.0.0.

Good point, I missed that indeed.

To avoid this in the future, either prefix the tags with a v or use a different branch name, e.g. 0_8_1 or release_0.8.1.

I thought this meant adding a v prefix would be a good idea? And indeed, I have used this system in other projects in collaboration with others where we are more careful with versions etc...

@penguinpee
Copy link
Contributor Author

To avoid this in the future, either prefix the tags with a v or use a different branch name, e.g. 0_8_1 or release_0.8.1.

I thought this meant adding a v prefix would be a good idea? And indeed, I have used this system in other projects in collaboration with others where we are more careful with versions etc...

Indeed, this was one of the options I suggested. Both will work. It's not a question of right or wrong. In these circumstances, I, personally, would have gone for a different branch name. Branches are bit more flexible with regards to renames.

But if you stick with the v prefix going forward and, maybe, switch to semantic versioning internally and when tagging, all will be fine. I already adjusted our tooling and test built the new release. Now waiting for bioframe to be reviewed...

@Phlya
Copy link
Owner

Phlya commented Jan 11, 2024

Someone else asked to review whether depending on bioframe is necessary, and I just copied over two necessary functions from there. So you don't need to wait for the bioframe review...

I released some new versions, only the latest one (1.0.3) is packaged correctly :)

@penguinpee
Copy link
Contributor Author

Someone else asked to review whether depending on bioframe is necessary, and I just copied over two necessary functions from there. So you don't need to wait for the bioframe review...

Oh well, it just got reviewed and is now available in Fedora's rawhide repo. I'll keep it for now. If it turns out to be a burden I can always retire it again later. But thanks for looking into it and making the required changes dropping bioframe as a dependency.

I released some new versions, only the latest one (1.0.3) is packaged correctly :)

As the saying goes: the third time is the charm. ;)

Or maybe not... (see #168)

@Phlya
Copy link
Owner

Phlya commented Jan 13, 2024

Oh well, that's embarassing! I noticed this import was a problem for building the docs, so I removed it - should have realized it's an issue in general! I'll make a new minor release now... Thank you for letting me know!

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

No branches or pull requests

3 participants