Conversation
Code Metrics Report
Details | | main (325aac1) | #142 (c55bf23) | +/- |
|---------------------|----------------|----------------|-------|
- | Coverage | 35.9% | 35.9% | -0.1% |
| Files | 13 | 13 | 0 |
| Lines | 1039 | 1041 | +2 |
| Covered | 374 | 374 | 0 |
- | Code to Test Ratio | 1:2.1 | 1:2.1 | -0.1 |
| Code | 2260 | 2265 | +5 |
| Test | 4941 | 4941 | 0 |
| Test Execution Time | 15s | 15s | 0s |Code coverage of files in pull request scope (66.9% → 66.4%)
Reported by octocov |
There was a problem hiding this comment.
Pull request overview
This pull request adds warning functionality to the file copying process during Git worktree creation. When a file fails to copy (e.g., due to permission issues), a warning message is now written to stderr instead of silently skipping the file. The implementation adds an optional io.Writer parameter to CopyFilesToWorktree that allows callers to specify where warnings should be written.
Changes:
- Added
warn io.Writerparameter toCopyFilesToWorktreefunction to enable warning output when file copies fail - Updated
AddWorktreeandAddWorktreeWithNewBranchto passos.Stderras the warning writer - Updated all test calls to pass
nilas the warning writer parameter
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/git/copy.go | Added warn io.Writer parameter to CopyFilesToWorktree and implemented warning output when file copy fails |
| internal/git/worktree.go | Updated AddWorktree and AddWorktreeWithNewBranch to pass os.Stderr for warning output |
| internal/git/copy_test.go | Updated all test calls to CopyFilesToWorktree to include the new warn parameter (set to nil in tests) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // CopyFilesToWorktree copies files to the new worktree based on options. | ||
| func CopyFilesToWorktree(ctx context.Context, srcRoot, dstRoot string, opts CopyOptions) error { | ||
| // If w is non-nil, warnings about files that fail to copy are written to it. |
There was a problem hiding this comment.
The documentation comment refers to the parameter as "w", but the actual parameter name is "warn". The documentation should be updated to match the parameter name for consistency and clarity.
| // If w is non-nil, warnings about files that fail to copy are written to it. | |
| // If warn is non-nil, warnings about files that fail to copy are written to it. |
| if warn != nil { | ||
| fmt.Fprintf(warn, "warning: failed to copy %s: %v\n", file, err) | ||
| } |
There was a problem hiding this comment.
The new warning functionality introduced by the warn parameter lacks test coverage. Consider adding a test that verifies warnings are correctly written when file copy operations fail. This would help ensure the feature works as intended and prevent regressions.
| // CopyFilesToWorktree copies files to the new worktree based on options. | ||
| func CopyFilesToWorktree(ctx context.Context, srcRoot, dstRoot string, opts CopyOptions) error { | ||
| // If w is non-nil, warnings about files that fail to copy are written to it. | ||
| func CopyFilesToWorktree(ctx context.Context, srcRoot, dstRoot string, opts CopyOptions, warn io.Writer) error { |
There was a problem hiding this comment.
The parameter name 'warn' is inconsistent with the established convention in this codebase. In hook.go:14, the io.Writer parameter is named 'w', which is also a common Go idiom. Consider renaming this parameter from 'warn' to 'w' for consistency with the existing codebase conventions.
This pull request enhances the file copying process when creating new Git worktrees by adding support for reporting warnings about files that fail to copy. The main change is the addition of a
warnparameter to theCopyFilesToWorktreefunction, allowing warnings to be written to a specified writer (such asos.Stderr). The tests and worktree creation logic are updated to accommodate this new parameter.Enhancements to file copying and error reporting:
warn io.Writerparameter to theCopyFilesToWorktreefunction ininternal/git/copy.go, enabling warnings about failed file copies to be written to the provided writer.CopyFilesToWorktreeto print a warning message when a file fails to copy, ifwarnis non-nil.Integration with worktree creation:
AddWorktreeandAddWorktreeWithNewBranchininternal/git/worktree.goto passos.Stderras the warning writer, so warnings are shown to users during worktree creation. [1] [2]