Replace release.sh with Python implementation using Typer and loguru#286
Replace release.sh with Python implementation using Typer and loguru#286
Conversation
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
…lect path changes
|
@copilot: use a typer interface at main |
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Signed-off-by: Thomas Schmelzer <thomas.schmelzer@gmail.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Signed-off-by: Thomas Schmelzer <thomas.schmelzer@gmail.com>
Signed-off-by: Thomas Schmelzer <thomas.schmelzer@gmail.com>
|
@copilot address the codefactor issue |
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Done in a5b835d. Refactored |
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Signed-off-by: Thomas Schmelzer <thomas.schmelzer@gmail.com>
|
@HarryCampion: Please try to merge. Away now |
There was a problem hiding this comment.
Pull request overview
This pull request replaces the shell script release.sh with a Python implementation release.py using the Typer CLI framework. The new implementation maintains all existing functionality including tag creation, validation checks, and prompts, while adding a new --dry-run option to preview changes without executing them.
Changes:
- Replaced POSIX shell script with Python implementation using Typer for CLI interface
- Added
--dry-runflag for simulating releases without making changes - Updated all test files to run Python script instead of shell script
- Removed obsolete tests for shell script executability
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
.rhiza/scripts/release.sh |
Removed the entire shell script implementation (235 lines deleted) |
.rhiza/scripts/release.py |
New Python implementation with Typer CLI, all original functionality preserved plus dry-run support |
tests/test_rhiza/test_release_script.py |
Updated to run Python script with sys.executable instead of shell |
tests/test_rhiza/conftest.py |
Updated to copy release.py instead of release.sh and fixed comment reference |
tests/test_rhiza/test_git_repo_fixture.py |
Removed obsolete tests checking for release.sh existence and executability |
| # Show commits since last tag | ||
| last_tag = get_last_tag() | ||
| if last_tag and last_tag != tag: | ||
| commit_count = count_commits_since_tag(last_tag, "HEAD") | ||
| print(f"Commits since {last_tag}: {commit_count}") |
There was a problem hiding this comment.
When an existing tag is being pushed (skip_tag_create is True), the commit count is calculated from last_tag to HEAD. However, the existing tag might not point to HEAD. It would be more accurate to count commits from last_tag to the tag being pushed. Consider passing the tag parameter to count_commits_since_tag(last_tag, tag) instead of using "HEAD", or document that this shows commits since the last tag to the current HEAD rather than to the tag being pushed.
| # Show commits since last tag | |
| last_tag = get_last_tag() | |
| if last_tag and last_tag != tag: | |
| commit_count = count_commits_since_tag(last_tag, "HEAD") | |
| print(f"Commits since {last_tag}: {commit_count}") | |
| # Show commits between the last tag and the tag being pushed | |
| last_tag = get_last_tag() | |
| if last_tag and last_tag != tag: | |
| commit_count = count_commits_since_tag(last_tag, tag) | |
| print(f"Commits from {last_tag} to {tag}: {commit_count}") |
| def prompt_continue(message: str) -> bool: | ||
| """Prompt user to continue with an operation. | ||
|
|
||
| Args: | ||
| message: Message to display to user (optional) | ||
|
|
||
| Returns: | ||
| True if user wants to continue, False otherwise | ||
| """ |
There was a problem hiding this comment.
The docstring states "message: Message to display to user (optional)" but the parameter is not actually optional - it has no default value. Either add a default value of "" or update the docstring to remove "(optional)".
.rhiza/scripts/release.py
Outdated
| answer = input().strip().lower() | ||
| if answer in ("y", "yes"): | ||
| return True | ||
| print_colored(Colors.YELLOW, "[INFO] Aborted by user") |
There was a problem hiding this comment.
The prompt_continue function prints "Aborted by user" and returns False when the user declines, but the callers then raise typer.Exit(0) to exit. This creates redundant "Aborted by user" messages. Consider either removing the message from prompt_continue and letting callers handle the message, or having prompt_continue raise the Exit directly. The current pattern results in the message being printed even when the caller might want to handle the declined prompt differently.
| print_colored(Colors.YELLOW, "[INFO] Aborted by user") |
| def get_current_branch() -> str: | ||
| """Get the current git branch name. | ||
|
|
||
| Returns: | ||
| Branch name | ||
|
|
||
| Raises: | ||
| SystemExit: If branch cannot be determined | ||
| """ | ||
| try: | ||
| result = run_command(["git", "rev-parse", "--abbrev-ref", "HEAD"]) | ||
| branch = result.stdout.strip() | ||
| if not branch: | ||
| raise ValueError("Empty branch name") | ||
| return branch | ||
| except (subprocess.CalledProcessError, ValueError) as e: | ||
| print_colored(Colors.RED, f"[ERROR] Could not determine current branch: {e}") | ||
| raise typer.Exit(1) | ||
|
|
There was a problem hiding this comment.
The docstring states "Raises: SystemExit" but the code actually raises "typer.Exit(1)". Update the docstring to accurately reflect that it raises typer.Exit.
| def get_default_branch() -> str: | ||
| """Get the default branch from remote. | ||
|
|
||
| Returns: | ||
| Default branch name | ||
|
|
||
| Raises: | ||
| SystemExit: If default branch cannot be determined | ||
| """ | ||
| try: | ||
| result = run_command(["git", "remote", "show", "origin"]) | ||
| for line in result.stdout.splitlines(): | ||
| if "HEAD branch" in line: | ||
| branch = line.split()[-1] | ||
| if branch: | ||
| return branch | ||
| raise ValueError("Could not parse default branch") | ||
| except (subprocess.CalledProcessError, ValueError) as e: | ||
| print_colored(Colors.RED, f"[ERROR] Could not determine default branch from remote: {e}") | ||
| raise typer.Exit(1) | ||
|
|
There was a problem hiding this comment.
The docstring states "Raises: SystemExit" but the code actually raises "typer.Exit(1)". Update the docstring to accurately reflect that it raises typer.Exit.
| def check_working_tree_clean() -> None: | ||
| """Check if the working tree is clean. | ||
|
|
||
| Raises: | ||
| SystemExit: If there are uncommitted changes | ||
| """ | ||
| result = run_command(["git", "status", "--porcelain"]) | ||
| if result.stdout.strip(): | ||
| print_colored(Colors.RED, "[ERROR] You have uncommitted changes:") | ||
| run_command(["git", "status", "--short"], capture_output=False) | ||
| print_colored(Colors.RED, "\n[ERROR] Please commit or stash your changes before releasing.") | ||
| raise typer.Exit(1) | ||
|
|
There was a problem hiding this comment.
The docstring states "Raises: SystemExit" but the code actually raises "typer.Exit(1)". Update the docstring to accurately reflect that it raises typer.Exit.
| def check_remote_status(current_branch: str) -> None: | ||
| """Check if branch is in sync with remote. | ||
|
|
||
| Args: | ||
| current_branch: Name of the current branch | ||
|
|
||
| Raises: | ||
| SystemExit: If branch is behind remote or diverged | ||
| """ | ||
| print_colored(Colors.BLUE, "[INFO] Checking remote status...") | ||
|
|
||
| # Fetch latest changes from remote | ||
| result = run_command(["git", "fetch", "origin"], capture_output=True, check=False) | ||
| if result.returncode != 0: | ||
| print_colored(Colors.YELLOW, "[WARN] Failed to fetch from remote. Continuing with local information.") | ||
| # Continue anyway - the user might be offline or have auth issues but still want to proceed | ||
|
|
||
| # Get upstream branch | ||
| try: | ||
| result = run_command( | ||
| ["git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"], | ||
| check=False, | ||
| ) | ||
| if result.returncode != 0: | ||
| print_colored(Colors.RED, f"[ERROR] No upstream branch configured for {current_branch}") | ||
| raise typer.Exit(1) | ||
| upstream = result.stdout.strip() | ||
| except subprocess.CalledProcessError: | ||
| print_colored(Colors.RED, f"[ERROR] No upstream branch configured for {current_branch}") | ||
| raise typer.Exit(1) | ||
|
|
||
| # Get commit hashes | ||
| local_result = run_command(["git", "rev-parse", "@"]) | ||
| local = local_result.stdout.strip() | ||
|
|
||
| remote_result = run_command(["git", "rev-parse", upstream]) | ||
| remote = remote_result.stdout.strip() | ||
|
|
||
| base_result = run_command(["git", "merge-base", "@", upstream]) | ||
| base = base_result.stdout.strip() | ||
|
|
||
| # Check sync status | ||
| if local == remote: | ||
| # Up to date | ||
| return | ||
|
|
||
| if local == base: | ||
| # Local is behind remote | ||
| print_colored(Colors.RED, f"[ERROR] Your branch is behind '{upstream}'. Please pull changes.") | ||
| raise typer.Exit(1) | ||
| elif remote == base: | ||
| # Local is ahead of remote | ||
| print_colored(Colors.YELLOW, f"[WARN] Your branch is ahead of '{upstream}'.") | ||
| print("Unpushed commits:") | ||
| run_command(["git", "log", "--oneline", "--graph", "--decorate", f"{upstream}..HEAD"], capture_output=False) | ||
|
|
||
| if prompt_continue("Push changes to remote before releasing?"): | ||
| run_command(["git", "push", "origin", current_branch], capture_output=False) | ||
| else: | ||
| # Branches have diverged | ||
| print_colored(Colors.RED, f"[ERROR] Your branch has diverged from '{upstream}'. Please reconcile.") | ||
| raise typer.Exit(1) | ||
|
|
There was a problem hiding this comment.
The docstring states "Raises: SystemExit" but the code actually raises "typer.Exit(1)". Update the docstring to accurately reflect that it raises typer.Exit.
| run_command(["git", "log", "--oneline", "--graph", "--decorate", f"{upstream}..HEAD"], capture_output=False) | ||
|
|
||
| if prompt_continue("Push changes to remote before releasing?"): | ||
| run_command(["git", "push", "origin", current_branch], capture_output=False) |
There was a problem hiding this comment.
When the user declines to push changes to remote (prompt_continue returns False), the function continues execution without raising an exit. This means the script proceeds with creating/pushing the tag even though unpushed commits exist and the user declined to push them. This is inconsistent with other prompts where declining exits the process. Consider either exiting when the user declines, or documenting that this behavior is intentional (allowing users to release without pushing other commits).
| run_command(["git", "push", "origin", current_branch], capture_output=False) | |
| run_command(["git", "push", "origin", current_branch], capture_output=False) | |
| else: | |
| print_colored( | |
| Colors.RED, | |
| "[ERROR] Release aborted: branch is ahead of remote and changes were not pushed.", | |
| ) | |
| raise typer.Exit(1) |
| check_working_tree_clean() | ||
| check_remote_status(current_branch) |
There was a problem hiding this comment.
In dry-run mode, the script still calls check_working_tree_clean() and check_remote_status() which can exit the script or prompt the user for actions. For a true dry-run experience, these validation checks should either be skipped with a message indicating what would be checked, or the functions should accept a dry_run parameter to only report issues without prompting or exiting. Currently, dry-run mode can still fail due to uncommitted changes or prompt users to push commits, which is unexpected behavior for a dry-run.
| check_working_tree_clean() | |
| check_remote_status(current_branch) | |
| if dry_run: | |
| print_colored( | |
| Colors.YELLOW, | |
| "[DRY RUN] Would verify that the git working tree is clean (no uncommitted changes)", | |
| ) | |
| print_colored( | |
| Colors.YELLOW, | |
| "[DRY RUN] Would check that the local branch is up to date with its remote", | |
| ) | |
| else: | |
| check_working_tree_clean() | |
| check_remote_status(current_branch) |
| def get_version(uv_bin: str) -> str: | ||
| """Get the current version from pyproject.toml using uv. | ||
|
|
||
| Args: | ||
| uv_bin: Path to uv binary | ||
|
|
||
| Returns: | ||
| Version string | ||
|
|
||
| Raises: | ||
| SystemExit: If version cannot be determined | ||
| """ | ||
| try: | ||
| result = run_command([uv_bin, "version", "--short"]) | ||
| version = result.stdout.strip() | ||
| if not version: | ||
| raise ValueError("Empty version string") | ||
| return version | ||
| except (subprocess.CalledProcessError, ValueError) as e: | ||
| print_colored(Colors.RED, f"[ERROR] Could not determine version from pyproject.toml: {e}") | ||
| raise typer.Exit(1) | ||
|
|
There was a problem hiding this comment.
The docstring states "Raises: SystemExit" but the code actually raises "typer.Exit(1)" which is not the same as SystemExit. While typer.Exit may eventually cause a system exit, it's more accurate to document this as raising typer.Exit for consistency with the actual implementation.
|
@HarryCampion The idea is to get the code working here and then move it to rhiza-tools |
I can move over the weekend into rhiza-tools. Makes sense to do it in stages like this, easier to port and test |
|
@tschm might be better to just use loguru I think copilot is generating some colour print |
|
@copilot Don't mess with colors. Use loguru for logging. |
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Done in 650bcaa. Replaced custom color codes and |
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Co-authored-by: tschm <2046079+tschm@users.noreply.github.com>
Replace release.sh with Python Script
Completed:
The release script now has improved code quality with reduced complexity, consistent error messaging, and professional logging using loguru. CodeFactor rating: 10.00/10.
Logging
The script uses loguru for all output with automatic colorization:
logger.error()for error messages (red)logger.warning()for warnings (yellow)logger.info()for informational messages (white/blue)logger.success()for success messages (green)This provides cleaner code without manual color management and follows industry-standard logging practices.
Dependencies
typer==0.21.1- Modern CLI framework with auto-generated help and shell completionloguru==0.7.3- Professional logging with automatic colorizationOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.