Conversation
Signed-off-by: samzong <[email protected]>
![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 introduces logic to ensure that new worktrees are correctly configured when extensions.worktreeConfig is enabled, specifically by setting core.bare to false in the worktree-specific configuration. These changes are integrated into the AddPR, Add, and Dup methods, and are supported by new test cases for bare repository layouts. Feedback was provided regarding the efficiency of the Dup method, where calling ensureAddedWorktreeConfig inside a loop results in redundant Git process executions; it is recommended to check the repository-level configuration once or cache the result.
| if err := c.ensureAddedWorktreeConfig(targetPath); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Calling ensureAddedWorktreeConfig inside the loop in Dup is inefficient because it spawns a redundant git config process for every worktree created to check the repository-level extensions.worktreeConfig setting. Since this setting is constant for the repository during the operation, consider checking it once before the loop and only calling the configuration command if necessary, or caching the result in the Client instance.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96194aa831
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func (c *Client) ensureAddedWorktreeConfig(targetPath string) error { | ||
| if c.getGitOutput(c.repoDir, "config", "--bool", "extensions.worktreeConfig") != "true" { |
There was a problem hiding this comment.
The guard reads extensions.worktreeConfig without a scope, so a global value (git config --global extensions.worktreeConfig true) is treated as if the repository enabled the extension; in that case ensureAddedWorktreeConfig runs git config --worktree ... and Git errors (--worktree cannot be used with multiple working trees unless ... enabled), causing wt add, wt dup, and PR worktree creation to fail in repos that do not have the extension set locally. The check should be restricted to repository/local config (or the common repo config file) before using --worktree.
Useful? React with 👍 / 👎.
Signed-off-by: samzong <[email protected]>
1 participant
What's changed?
core.bare=falsein newly created linked worktrees whenextensions.worktreeConfigis enabled.wt add,wt dup, and PR worktree creation..bare + extensions.worktreeConfigand for bare layouts without worktree config.Why
core.bare=truefrom.bare/config, which made normal Git commands fail withfatal: this operation must be run in a work tree.Fixes #66