Add commit_id to dump_file template#1154
Conversation
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
i like the idea - please fix the tests and propose a change-log entry (i'll add it to the change-log)
Fixed the tests. I also renamed the new attributes to Proposed change-log entry: |
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
Thanks unfortunately it seems like that one test missed in naming
Perhaps i should have given the template in the test a more detailed name
I also took note that there's potential situations where nodeid isn't known
I believe those where the reason for not including it in the beginning
But I'm unaware of a easy acceptance test for that so i need to log a follow-up
| assert lines[-4:] == [ | ||
| "__version__ = version = '1.0.dev42'", | ||
| "__version_tuple__ = version_tuple = (1, 0, 'dev42')", | ||
| "", |
There was a problem hiding this comment.
This should get a different test
There was a problem hiding this comment.
I agree, I added a second test that doesn't use the test template but where the commit id is not None
testing/test_basic_api.py
Outdated
| __version__ = version = {version!r} | ||
| __version_tuple__ = version_tuple = {version_tuple!r} | ||
| __sha__ = {scm_version.node!r} | ||
| __sha__ = sha = {scm_version.node!r} |
There was a problem hiding this comment.
The template is intentionally different from the normal template
If its the same using it in test no longer shows different behavior
There was a problem hiding this comment.
Ah ok understood, I changed it back
| __version__: str | ||
| __version_tuple__: VERSION_TUPLE | ||
| version_tuple: VERSION_TUPLE | ||
| sha: str |
There was a problem hiding this comment.
Lets use the name node_id or commit_id
Just sha is actually incorrect for scm like jj pujil or darcs
There was a problem hiding this comment.
Alright, I changed it to commit_id
for more information, see https://pre-commit.ci
I changed the type of the attribute to |
|
well done, i'll merge + add a changelog entry after i finish sorting out my other pr |
Alright, thank you! |
For my use case I want to report not only the current version of my application but also the git hash at which it was built. setuptools-scm includes the git hash if the version isn't exact (i.e. the application wasn't built from a commit with a git tag), but not if it is. This behavior on it's own is fine and expected imo. However I still needed an easy way to access the git hash in these cases in addition to the version, so I maid this small adjustment to the dump_file template so that it always includes the current git_hash in addition the the version and version_tuple, regardless of whether the version or version_tuple includes the git hash already or not.
I am currently already using this change for my own packages project-W and project-W-runner, but maybe they are useful to other users as well so I decided to create this PR.