SNEWS2 / snewpy

A Python package for working with supernova neutrinos
https://snewpy.readthedocs.io
BSD 3-Clause "New" or "Revised" License
27 stars 22 forks source link

Adding version checks #368

Closed Sheshuk closed 6 hours ago

Sheshuk commented 2 days ago

This PR adds two new initial checks, to make sure that our versions are consistent in different places:

Sheshuk commented 2 days ago

Hm, checking the git tag doesn't work for some reason. I tried to use fetch-tags option for the checkout action, but it seems that it will need to checkout the whole git tree to get the tags (so, I'll need to set fetch-depth: 0). Investigating if that can be avoided...

Sheshuk commented 1 day ago

Looks like it doesn't work as intended: checkout action doesn't load the git tags (unless we tell it fetch-depth:0 to pull all the history and branches, which is probably not what we want).

Moreover, this test will fail when we prepare for a new release: first we change __version__, and then we cut a new tag after we merge.

This test might, on the other hand, enforce making git tag in the release branch before merging.

Also these could be separate tests, triggered when we make the release. Need to think about it...

JostMigenda commented 7 hours ago

ICYMI, the publish workflow is already checking that snewpy.__version__ matches the git tag: https://github.com/SNEWS2/snewpy/blob/1ad71b82e1bf037dce67df0bb074fe9a32a3752b/.github/workflows/publish.yml#L26-L35

If I understand correctly, importlib.metadata.version gets the version set in setup.py? That takes the version string from the same place as snewpy.__version__; so I think we’ve already guaranteed that those three are all in sync for published releases. And on non-release PRs, I’m not sure this check is really necessary—am I missing something?

Sheshuk commented 6 hours ago

Oh, right, thank you for pointing it out. I didn't know about this check. Then we're perfectly fine