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

Adding version checks #368

Closed
wants to merge 2 commits into from
Closed

Adding version checks #368

wants to merge 2 commits into from

Conversation

Sheshuk
Copy link
Contributor

@Sheshuk Sheshuk commented Dec 2, 2024

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

  • snewpy.__version__
  • importlib.metadata.version (the one set via pip install)
  • current git tag (git describe --abbrev=0), after removing the v prefix

@Sheshuk Sheshuk marked this pull request as draft December 2, 2024 15:01
@Sheshuk
Copy link
Contributor Author

Sheshuk commented Dec 2, 2024

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
Copy link
Contributor Author

Sheshuk commented Dec 3, 2024

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
Copy link
Member

ICYMI, the publish workflow is already checking that snewpy.__version__ matches the git tag:

- name: Check and get Version Number
id: get_version
run: |
pip install .
PYTHON_VERSION=`python -c 'import snewpy; print(snewpy.__version__)'`
echo "PYTHON_VERSION=${PYTHON_VERSION}"
GIT_VERSION=${GITHUB_REF/refs\/tags\/v/}
echo "GIT_VERSION=${GIT_VERSION}"
if [ $PYTHON_VERSION != $GIT_VERSION ]; then exit 1; fi
echo "VERSION=${PYTHON_VERSION}" >> $GITHUB_OUTPUT

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
Copy link
Contributor Author

Sheshuk commented Dec 4, 2024

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

@Sheshuk Sheshuk closed this Dec 4, 2024
@JostMigenda JostMigenda mentioned this pull request Dec 4, 2024
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.

2 participants