Add porcelain-style convenience APIs on Repo (create_branch and merge)#2120
Conversation
… and `merge`) Signed-off-by: Nguyen Huy Hoang <181364121+huyhoang171106@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds porcelain-style convenience APIs to git.repo.base.Repo to make common operations more ergonomic without requiring users to compose lower-level calls.
Changes:
- Add
Repo.create_branch()as an alias forRepo.create_head(). - Add
Repo.merge()as a convenience wrapper aroundrepo.git.merge().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def create_branch(self, *args: Any, **kwargs: Any) -> Head: | ||
| return self.create_head(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
create_branch is a new public API but currently just forwards *args/**kwargs with no docstring. That loses the explicit signature of create_head (path/commit/force/logmsg), weakens type checking and makes the API harder to discover in docs/IDE autocomplete. Consider giving create_branch the same explicit parameters and a short docstring that points to create_head/Head.create (and optionally place it next to create_head for discoverability).
| def create_branch(self, *args: Any, **kwargs: Any) -> Head: | |
| return self.create_head(*args, **kwargs) | |
| def create_branch( | |
| self, | |
| path: str, | |
| commit: Commit | str | None = "HEAD", | |
| force: bool = False, | |
| logmsg: str | None = None, | |
| ) -> Head: | |
| """Create a new branch pointing to the given commit. | |
| This is a convenience wrapper around :meth:`create_head` / | |
| :meth:`git.refs.Head.create`. | |
| """ | |
| return self.create_head(path, commit=commit, force=force, logmsg=logmsg) |
| def create_branch(self, *args: Any, **kwargs: Any) -> Head: | ||
| return self.create_head(*args, **kwargs) | ||
|
|
||
| def merge(self, *args: Any, **kwargs: Any) -> str: |
There was a problem hiding this comment.
merge is typed as returning str, but this is a raw passthrough to self.git.merge(*args, **kwargs). Callers can pass execution flags like as_process, with_extended_output, or stdout_as_string=False, which change the return type (process handle / bytes / tuple). Either constrain the wrapper to the porcelain use-case (no execution-control kwargs) or widen the return type accordingly so the annotation matches actual behavior.
| def merge(self, *args: Any, **kwargs: Any) -> str: | |
| def merge(self, *args: Any, **kwargs: Any) -> Any: |
| def merge(self, *args: Any, **kwargs: Any) -> str: | ||
| return self.git.merge(*args, **kwargs) |
There was a problem hiding this comment.
The PR/issue description suggests an ergonomic call like repo.merge(from=branch_name), but this wrapper doesn't introduce any named parameter (and Python can't accept a keyword named from). If the intent is a porcelain-style API, consider adding an explicit named argument (e.g., from_/other/rev) and mapping it to the positional merge target, rather than only exposing the low-level *args/**kwargs passthrough.
| def create_branch(self, *args: Any, **kwargs: Any) -> Head: | ||
| return self.create_head(*args, **kwargs) | ||
|
|
||
| def merge(self, *args: Any, **kwargs: Any) -> str: | ||
| return self.git.merge(*args, **kwargs) |
There was a problem hiding this comment.
New public Repo convenience methods are added here (create_branch, merge), but there are no tests ensuring they behave as intended (e.g., create_branch being an alias of create_head, and merge delegating to git merge successfully). Given the existing test coverage around create_head/repo operations, adding a small test would help prevent regressions and clarify expected behavior (including error behavior on conflicts).
Summary
Introduce higher-level, intuitive repository operations directly on
Repoto reduce reliance on plumbing-level sequences. This file is whereRepomethods live, including existing branch/head creation helpers, so it is the correct location for ergonomic aliases.Files changed
git/repo/base.py(modified)Testing
Closes #901