Skip to content

Add minimum git version requirement check #39

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.

Sign up for GitHub

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

Open
jovnc wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add minimum git version requirement check #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9791469
feat: add minimum git version requirement check
jovnc Jan 17, 2026
7d55b4b
chore: bump minimum git required version to 2.37.0
jovnc Jan 17, 2026
41d843c
chore: change to minimum version 2.28.0
jovnc Jan 17, 2026
34cecb9
chore: address comments from copilot
jovnc Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions app/commands/check/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import click

from app.utils.click import error, info, success
from app.utils.git import get_git_config, is_git_installed
from app.utils.git import (
MIN_GIT_VERSION,
get_git_config,
get_git_version,
)


@click.command()
Expand All @@ -10,11 +14,22 @@ def git() -> None:
Verifies if Git is installed and setup for Git-Mastery.
"""
info("Checking that you have Git installed and configured")
if is_git_installed():
info("Git is installed")
else:

git_version = get_git_version()
if git_version is None:
error("Git is not installed")

info("Git is installed")
Copy link

@desmondwong1215 desmondwong1215 Jan 24, 2026

Choose a reason for hiding this comment

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

Just a small nit. I am ok with omitting the else clause to make a happy path, but will it be better if we add it for clarity and consistency, which is similar to lines 34 - 41?

if not config_user_name:
        error(
            f"You do not have {click.style('user.name', bold=True)} yet. Run {click.style('git config --global user.name <name>', bold=True, italic=True)}."
        )
else:
        info(
            f"You have set {click.style('user.name', bold=True)} as {click.style(config_user_name, bold=True, italic=True)}"
        )

Copy link
Collaborator Author

@jovnc jovnc Jan 24, 2026

Choose a reason for hiding this comment

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

error() does a sys.exit(1) call which stops the entire program.

I believe that the else is redundant for the other lines of code as well, generally we should keep the happy path prominent and not hide it in else clauses.

Do you think it would be better for me to remove the else in other lines instead?

Copy link
Collaborator Author

@jovnc jovnc Jan 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

But again, generally we don't want to touch code that isn't broken, so it should be a small issue, whether we decide to include or omit the else


if git_version.is_behind(MIN_GIT_VERSION):
error(
f"Git {git_version} is behind the minimum required version. "
f"Please upgrade to Git {MIN_GIT_VERSION} or later. "
f"Refer to https://git-scm.com/downloads"
)

info(f"Git {git_version} meets the minimum version requirement.")

config_user_name = get_git_config("user.name")
if not config_user_name:
error(
Expand Down
19 changes: 17 additions & 2 deletions app/utils/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import re
from typing import Optional

from app.utils.command import run
from app.utils.version import Version

MIN_GIT_VERSION = Version(2, 28, 0)


def init() -> None:
Expand All @@ -23,11 +27,22 @@ def push(remote: str, branch: str) -> None:
run(["git", "push", "-u", remote, branch])


def is_git_installed() -> bool:
def get_git_version() -> Optional[Version]:
"""Get the installed git version.

Returns None if git is not installed or version cannot be parsed.
"""
# If git is not installed yet, we should expect a 127 exit code
# 127 indicating that the command not found: https://stackoverflow.com/questions/1763156/127-return-code-from
result = run(["git", "--version"])
return result.is_success()
if not result.is_success():
return None

match = re.search(r"^git version (\d+\.\d+\.\d+)", result.stdout)
if not match:
return None

return Version.parse(match.group(1))


def remove_remote(remote: str) -> None:
Expand Down
17 changes: 17 additions & 0 deletions app/utils/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,27 @@ class Version:

@staticmethod
def parse_version_string(version: str) -> "Version":
"""Parse a version string with 'v' prefix (e.g., 'v1.2.3')."""
only_version = version[1:]
[major, minor, patch] = only_version.split(".")
return Version(int(major), int(minor), int(patch))

@staticmethod
def parse(version: str) -> "Version":
"""Parse a plain version string (e.g., '1.2.3')."""
parts = version.split(".")
if len(parts) != 3:
raise ValueError(
f"Invalid version string (expected 'MAJOR.MINOR.PATCH'): {version!r}"
)
try:
major, minor, patch = (int(part) for part in parts)
except ValueError as exc:
raise ValueError(
f"Invalid numeric components in version string: {version!r}"
) from exc
return Version(major, minor, patch)

def is_behind(self, other: "Version") -> bool:
"""Returns if the current version is behind the other version based on major and minor versions."""
return (other.major, other.minor) > (self.major, self.minor)
Expand Down