bump go-git to v6#759
Conversation
Current Aviator status
This PR was merged using Aviator (commit 3fd7a2d).
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the go-git dependency from v5 to v6 across the codebase, updating imports and dependencies in go.mod and go.sum. To comply with go-git v6's resource management requirements, a Close method was added to the Repo struct, and a closeRepo helper was introduced and deferred in main.go via a new run function wrapper. Additionally, several fields in the sequencer were refactored from plumbing.Hash to string. The review feedback recommends resetting cachedRepo to nil after closing it to prevent failures in multi-command execution environments, and adding defensive nil checks in Repo.Close to avoid potential nil-pointer dereferences.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func closeRepo() { | ||
| if cachedRepo != nil { | ||
| _ = cachedRepo.Close() | ||
| } | ||
| } |
There was a problem hiding this comment.
In a multi-command execution environment (such as integration tests running multiple commands in the same process), cachedRepo is not reset to nil after being closed. This causes subsequent commands to retrieve the already-closed cachedRepo from getRepo(), leading to failures. Setting cachedRepo = nil after closing it ensures that subsequent calls can correctly reopen the repository.
| func closeRepo() { | |
| if cachedRepo != nil { | |
| _ = cachedRepo.Close() | |
| } | |
| } | |
| func closeRepo() { | |
| if cachedRepo != nil { | |
| _ = cachedRepo.Close() | |
| cachedRepo = nil | |
| } | |
| } |
| func (r *Repo) Close() error { | ||
| return r.gitRepo.Close() | ||
| } |
There was a problem hiding this comment.
To prevent potential nil-pointer dereferences, it is safer to add a nil check for both the receiver r and the underlying gitRepo before calling Close(). This aligns with defensive programming best practices.
func (r *Repo) Close() error {
if r == nil || r.gitRepo == nil {
return nil
}
return r.gitRepo.Close()
}
There was a problem hiding this comment.
That's probably a good comment
✅ FlexReview StatusCommon Owner:
Review SLO: |
| func (r *Repo) Close() error { | ||
| return r.gitRepo.Close() | ||
| } |
There was a problem hiding this comment.
That's probably a good comment
follow-up to #759, defensive nil guard on Repo.Close per review
2 participants
trying go-git v6 while it's still in alpha