Conversation
…ing, and enhance linting rules
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors Git and GitHub-related functionality by introducing a new Constants module for centralized configuration, adding Git tag handling capabilities, and updating the changelog management to remove dependency on environment variables. The changes also include linting rule adjustments and test updates to align with the new architecture.
- Introduces a centralized Constants module with GitConstants for GitHub URLs
- Adds comprehensive Git tag functionality with version comparison and parsing
- Removes get_github_repo() function in favor of static constants
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/devops/config/constants.py |
Adds new Constants and GitConstants classes for centralized GitHub URL management |
src/devops/config/__init__.py |
Exports the new Constants class |
src/devops/git/tag.py |
Introduces GitTag dataclass and functions for Git tag parsing, comparison, and retrieval |
src/devops/git/__init__.py |
Adds module docstring for Git-related functionality |
src/devops/git/github.py |
Removes get_github_repo() function that relied on environment variables |
src/devops/files/update_changelog.py |
Updates to use Constants instead of get_github_repo(), renames exception from MSTD to DevOps |
src/devops/utils/utils.py |
Updates error message to reference Constants.github.github_devops_issues_url |
src/devops/enums/__init__.py |
Adds LogLevel to exports |
tests/files/test_update_changelog.py |
Updates tests to remove mocking of get_github_repo() and use Constants, renames exception references |
ruff.toml |
Moves PLR2004 (magic numbers) rule from test-specific ignores to global ignores |
.gitignore |
Adds specific test notebook file to ignore list |
Comments suppressed due to low confidence (1)
src/devops/config/constants.py:3
- Import of 'dataclass' is not used.
from dataclasses import dataclass
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…e-last-release-and-or-tag-from-git-github
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/devops/git/tag.py
Outdated
Comment on lines
+1
to
+241
| """Module for Git tag-related constants and functions.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import subprocess | ||
| from dataclasses import dataclass | ||
|
|
||
|
|
||
| # TODO(97gamjak): centralize exception handling | ||
| # https://github.com/97gamjak/devops/issues/24 | ||
| class GitTagError(Exception): | ||
| """Exception raised for Git tag-related errors in devops checks.""" | ||
|
|
||
| def __init__(self, message: str) -> None: | ||
| """Initialize the exception with a message.""" | ||
| super().__init__(f"GitTagError: {message}") | ||
| self.message = message | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class GitTag: | ||
| """Class representing a Git tag.""" | ||
|
|
||
| major: int = 0 | ||
| minor: int = 0 | ||
| patch: int = 0 | ||
|
|
||
| def __str__(self) -> str: | ||
| """Return the string representation of the Git tag. | ||
|
|
||
| Returns | ||
| ------- | ||
| str | ||
| The string representation of the Git tag | ||
| in the format 'v<major>.<minor>.<patch>'. | ||
|
|
||
| """ | ||
| return f"v{self.major}.{self.minor}.{self.patch}" | ||
|
|
||
| def __eq__(self, other: object) -> bool: | ||
| """Check equality between two GitTag instances. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| other: object | ||
| The other GitTag instance to compare with. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if both GitTag instances are equal, False otherwise. | ||
|
|
||
| """ | ||
| if not isinstance(other, GitTag): | ||
| return NotImplemented | ||
| return (self.major, self.minor, self.patch) == ( | ||
| other.major, | ||
| other.minor, | ||
| other.patch, | ||
| ) | ||
|
|
||
| def __hash__(self) -> int: | ||
| """Return the hash of the GitTag instance. | ||
|
|
||
| Returns | ||
| ------- | ||
| int | ||
| The hash value of the GitTag instance. | ||
|
|
||
| """ | ||
| return hash((self.major, self.minor, self.patch)) | ||
|
|
||
| def __lt__(self, other: GitTag) -> bool: | ||
| """Check if this GitTag is less than another GitTag. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| other: GitTag | ||
| The other GitTag instance to compare with. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if this GitTag is less than the other GitTag, | ||
| False otherwise. | ||
|
|
||
| """ | ||
| version_self = (self.major, self.minor, self.patch) | ||
| version_other = (other.major, other.minor, other.patch) | ||
| return version_self < version_other | ||
|
|
||
| def __gt__(self, other: GitTag) -> bool: | ||
| """Check if this GitTag is greater than another GitTag. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| other: GitTag | ||
| The other GitTag instance to compare with. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if this GitTag is greater than the other GitTag, | ||
| False otherwise. | ||
|
|
||
| """ | ||
| version_self = (self.major, self.minor, self.patch) | ||
| version_other = (other.major, other.minor, other.patch) | ||
| return version_self > version_other | ||
|
|
||
| def __le__(self, other: GitTag) -> bool: | ||
| """Check if this GitTag is less than or equal to another GitTag. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| other: GitTag | ||
| The other GitTag instance to compare with. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if this GitTag is less than or equal to the other GitTag, | ||
| False otherwise. | ||
|
|
||
| """ | ||
| version_self = (self.major, self.minor, self.patch) | ||
| version_other = (other.major, other.minor, other.patch) | ||
| return version_self <= version_other | ||
|
|
||
| def __ge__(self, other: GitTag) -> bool: | ||
| """Check if this GitTag is greater than or equal to another GitTag. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| other: GitTag | ||
| The other GitTag instance to compare with. | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if this GitTag is greater than or equal to the other GitTag, | ||
| False otherwise. | ||
|
|
||
| """ | ||
| version_self = (self.major, self.minor, self.patch) | ||
| version_other = (other.major, other.minor, other.patch) | ||
| return version_self >= version_other | ||
|
|
||
| @staticmethod | ||
| def from_string(tag: str) -> GitTag: | ||
| """Create a GitTag instance from a string. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| tag: str | ||
| The Git tag string in the format 'v<major>.<minor>.<patch>'. | ||
|
|
||
| Returns | ||
| ------- | ||
| GitTag | ||
| The GitTag instance created from the string. | ||
|
|
||
| Raises | ||
| ------ | ||
| GitTagError | ||
| If the tag string is not in the correct format. | ||
|
|
||
| """ | ||
| tag = tag.removeprefix("v") | ||
| parts = tag.split(".") | ||
|
|
||
| # TODO(97gamjak): implement support for different version schemes | ||
| # https://97gamjak.atlassian.net/browse/DEV-49 | ||
| if len(parts) != 3: # noqa: PLR2004 this will be removed and cleaned up with further naming schemes | ||
| msg = f"Invalid tag format: {tag}" | ||
| raise GitTagError(msg) | ||
|
|
||
| try: | ||
| major, minor, patch = map(int, parts) | ||
| except ValueError as exc: | ||
| msg = f"Invalid numeric components in tag: {tag}" | ||
| raise GitTagError(msg) from exc | ||
| return GitTag(major, minor, patch) | ||
|
|
||
|
|
||
| def get_all_tags(*, empty_tag_list_allowed: bool = True) -> list[GitTag]: | ||
| """Get all Git tags in the repository. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| empty_tag_list_allowed: bool | ||
| Whether to allow an empty tag list without raising an error. | ||
|
|
||
| Returns | ||
| ------- | ||
| list[GitTag] | ||
| A list of all Git tags. | ||
|
|
||
| Raises | ||
| ------ | ||
| GitTagError | ||
| If there is an error retrieving the Git tags and | ||
| empty_tag_list_allowed is False. | ||
|
|
||
| """ | ||
| try: | ||
| tags_output = subprocess.check_output( | ||
| ["git", "tag", "--list"], | ||
| stderr=subprocess.DEVNULL, | ||
| text=True, | ||
| ).strip() | ||
| except subprocess.CalledProcessError as e: | ||
| if not empty_tag_list_allowed: | ||
| msg = "Failed to retrieve Git tags." | ||
| raise GitTagError(msg) from e | ||
| return [] | ||
|
|
||
| tags = [] | ||
| for tag_str in tags_output.splitlines(): | ||
| tag = GitTag.from_string(tag_str) | ||
| tags.append(tag) | ||
|
|
||
| return tags | ||
|
|
||
|
|
||
| def get_latest_tag() -> GitTag: | ||
| """Get the latest Git tag in the repository. | ||
|
|
||
| Returns | ||
| ------- | ||
| GitTag | ||
| The latest Git tag. If no tags exist, returns GitTag(0, 0, 0). | ||
|
|
||
| """ | ||
| tags = get_all_tags() | ||
| if not tags: | ||
| return GitTag(0, 0, 0) | ||
|
|
||
| sorted_tags = sorted(tags, reverse=True) | ||
|
|
||
| return sorted_tags[0] |
There was a problem hiding this comment.
The new Git tag functionality in src/devops/git/tag.py lacks test coverage. Other modules in the project, such as src/devops/files/update_changelog.py and src/devops/config/config.py, have corresponding test files. The GitTag class and functions like get_all_tags and get_latest_tag should have comprehensive unit tests to cover tag parsing, comparison operations, and edge cases (e.g., empty tag lists, invalid tag formats).
Owner
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…t-github' of github.com:97gamjak/devops into feature/DEV-21-retrieve-last-release-and-or-tag-from-git-github
Contributor
Add 39 comprehensive unit tests for src/devops/git/tag.py covering: - GitTag class initialization and string representation - GitTag.from_string() with valid and invalid inputs - GitTag ordering and comparison operations - get_all_tags() with various scenarios and edge cases - get_latest_tag() with version ordering logic - GitTagError exception handling All tests pass and follow project conventions. Co-authored-by: 97gamjak <77228802+97gamjak@users.noreply.github.com>
Add test coverage for git tag module
…-related tests module
97gamjak
deleted the
feature/DEV-21-retrieve-last-release-and-or-tag-from-git-github
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ing, and enhance linting rules