unique version for each commit #409
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the version management system to generate unique versions for each commit using setuptools-scm. The changes move from a manually managed version string to an automated system that combines a base version from a file with the Git commit count.
Key changes:
- Replaced hardcoded version management with setuptools-scm integration
- Automated version generation using base version + commit count
- Consolidated version handling logic across build and versioning scripts
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Added setuptools-scm configuration with custom version scheme using base version + Git commit count |
| pyrx/init.py | Changed from hardcoded version string to importing from generated _version.py file |
| pyproject.toml | Updated build requirements to include setuptools-scm and configured SCM settings |
| Version/version.txt | Added base version file containing the semantic version base (2.2.23) |
| Version/version.py | Refactored version management script to use new base version + commit count approach |
| Version/init.tpl | Removed template file that is no longer needed with the new version system |
|
|
||
| def get_revision_number() -> str: | ||
| number = subprocess.check_output( | ||
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True |
There was a problem hiding this comment.
The subprocess call does not specify a working directory and could potentially execute from an unexpected location. Consider adding the cwd parameter to ensure the git command runs from the correct repository directory.
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True | |
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True, cwd=BASE_DIR |
| base_version = get_base_version() | ||
| try: | ||
| revision = get_revision_number() | ||
| except Exception: |
There was a problem hiding this comment.
Catching all exceptions with except Exception: is too broad. Consider catching specific exceptions like subprocess.CalledProcessError or FileNotFoundError to handle known failure cases appropriately.
| except Exception: | |
| except (subprocess.CalledProcessError, FileNotFoundError, OSError): |
|
|
||
| def get_revision_number() -> str: | ||
| number = subprocess.check_output( | ||
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True |
There was a problem hiding this comment.
The subprocess call does not specify a working directory. This could lead to unexpected behavior if the script is run from a different directory. Consider adding cwd=REPO_DIR parameter to ensure consistency.
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True | |
| ["git", "rev-list", "--count", "--first-parent", "main"], text=True, cwd=str(REPO_DIR) |
| version_scheme=custom_version_scheme, | ||
| local_scheme="no-local-version", | ||
| ) | ||
| def parse_version(version: str) -> tuple: |
There was a problem hiding this comment.
The return type annotation tuple is too generic. Consider using tuple[str, str, str] to specify that it returns a 3-tuple of strings representing major, minor, and revision components.
| def parse_version(version: str) -> tuple: | |
| def parse_version(version: str) -> tuple[str, str, str]: |
|
Sorry, I don’t want |
unique version for each commit
No description provided.