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 versionstring from sydent/snapse to common #22

Merged
merged 10 commits into from
Feb 11, 2022
Merged

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Feb 11, 2022

Fixes #6.

Suggest reviewing commit-by-commit.

Rather than assuming __version__ exists, use importlib to grab package metadata. (I've done this so we don't have to duplicate the version in the source code and in the packaging config.)

>>> from matrix_common.versionstring import *
>>> get_distribution_version_string("matrix-common")
'1.0.0 (b=dmr/versionstring,8420421)'

Once merged, I'll make a minor release of the library, change synapse and sydent to require that version, and have them use the implementation in the PR.

David Robertson added 3 commits February 10, 2022 18:50
and clarify that we're looking for the version of a _distribution_
package.
@DMRobertson DMRobertson requested a review from a team as a code owner February 11, 2022 18:56
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty great overall! I left some questions but they're mostly me not really being familiar with importlib!

matrix_common/versionstring.py Outdated Show resolved Hide resolved
version_string = module.__version__ # type: ignore[attr-defined]
distribution = Distribution.from_name(distribution_name)
version_string = distribution.version
cwd = distribution.locate_file(".")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is locate_file documented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in the stdlib docs (which are surprisingly terse?). It is listed in the backport's API reference though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'm not even sure if it is a backport. Maybe it's a bit like pip --- an external package later vendored into the stdlib?

matrix_common/versionstring.py Outdated Show resolved Hide resolved
@DMRobertson DMRobertson changed the title Dmr/versionstring Move versionstring from sydent/snapse to common Feb 11, 2022
tests/test_versionstring.py Outdated Show resolved Hide resolved
DMRobertson and others added 2 commits February 11, 2022 19:37
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
matrix_common/versionstring.py Outdated Show resolved Hide resolved
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.

Factor out get_version_string
2 participants