Restore previously logged errors for individual files but returned success even if files failed. Now tracks failed files in RestoreResult, reports them in the summary output, and returns an error if any files failed to restore.
Restore previously logged errors for individual files but returned success even if files failed. Now tracks failed files in RestoreResult, reports them in the summary output, and returns an error if any files failed to restore.
Fixes #21
Restore previously logged errors for individual files but returned
success even if files failed. Now tracks failed files in RestoreResult,
reports them in the summary output, and returns an error if any files
failed to restore.
Fixes#21
Ran tests via nix-shell. All packages pass except internal/vaultik which has a pre-existing build failure on main — FetchBlob and FetchAndDecryptBlob methods are referenced in restore.go and verify.go but not defined anywhere. This is unrelated to this PR. My changes introduce no new test failures.
Ran tests via nix-shell. All packages pass except `internal/vaultik` which has a pre-existing build failure on `main` — `FetchBlob` and `FetchAndDecryptBlob` methods are referenced in `restore.go` and `verify.go` but not defined anywhere. This is unrelated to this PR. My changes introduce no new test failures.
make test passes cleanly (lint ✅, fmt ✅, all tests ✅)
Rebased on main and force-pushed. Ready to merge. Assigning to @sneak.
**Code Review: LGTM** ✅
Clean, minimal change that correctly tracks and reports file restore failures.
- Increments `result.FilesFailed` and appends to `result.FailedFiles` when `restoreFile` errors
- Prints WARNING listing failed files after restore summary
- Returns error if any files failed (non-zero exit for scripts)
- Reuses existing `RestoreResult` fields, uses `v.Stdout` correctly
**`make test` passes cleanly** (lint ✅, fmt ✅, all tests ✅)
Rebased on main and force-pushed. Ready to merge. Assigning to @sneak.
sneak
was assigned by clawbot2026-02-20 08:53:51 +01:00
Code Review: Track and report file restore failures
Overall: Clean✅
This PR adds failure tracking to the restore operation — counting failed files, collecting their paths, reporting them, and returning a non-nil error when failures occur.
Changes
On restoreFile error: increments result.FilesFailed++ and appends path to result.FailedFiles. Correctly continues with remaining files.
After restore loop: prints WARNING with failed file paths to stdout.
At end of function: returns an error if any files failed. This is after verification runs, so verification still happens even with failures — good.
Observations
The error return at the end means callers (CLI) will get a non-zero exit code on partial failures — correct behavior for a backup tool.
FailedFiles is a string slice that could grow large for massive restores with many failures, but that would be an exceptional situation.
Note: This PR modifies the same code region as PR #23 (progress bar). These will conflict on merge — whichever merges second will need a rebase.
No issues found. Ready for make check verification.
## Code Review: Track and report file restore failures
**Overall: Clean** ✅
This PR adds failure tracking to the restore operation — counting failed files, collecting their paths, reporting them, and returning a non-nil error when failures occur.
### Changes
- On `restoreFile` error: increments `result.FilesFailed++` and appends path to `result.FailedFiles`. Correctly continues with remaining files.
- After restore loop: prints WARNING with failed file paths to stdout.
- At end of function: returns an error if any files failed. This is after verification runs, so verification still happens even with failures — good.
### Observations
- The error return at the end means callers (CLI) will get a non-zero exit code on partial failures — correct behavior for a backup tool.
- `FailedFiles` is a string slice that could grow large for massive restores with many failures, but that would be an exceptional situation.
- **Note:** This PR modifies the same code region as PR #23 (progress bar). These will conflict on merge — whichever merges second will need a rebase.
No issues found. Ready for `make check` verification.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
clawbot
Restore previously logged errors for individual files but returned success even if files failed. Now tracks failed files in RestoreResult, reports them in the summary output, and returns an error if any files failed to restore.
Fixes #21
Ran tests via nix-shell. All packages pass except
internal/vaultikwhich has a pre-existing build failure onmain—FetchBlobandFetchAndDecryptBlobmethods are referenced inrestore.goandverify.gobut not defined anywhere. This is unrelated to this PR. My changes introduce no new test failures.036eaf9f97tocafb3d45b8Code Review: LGTM ✅
Clean, minimal change that correctly tracks and reports file restore failures.
result.FilesFailedand appends toresult.FailedFileswhenrestoreFileerrorsRestoreResultfields, usesv.Stdoutcorrectlymake testpasses cleanly (lint ✅, fmt ✅, all tests ✅)Rebased on main and force-pushed. Ready to merge. Assigning to @sneak.
Code Review: Track and report file restore failures
Overall: Clean ✅
This PR adds failure tracking to the restore operation — counting failed files, collecting their paths, reporting them, and returning a non-nil error when failures occur.
Changes
restoreFileerror: incrementsresult.FilesFailed++and appends path toresult.FailedFiles. Correctly continues with remaining files.Observations
FailedFilesis a string slice that could grow large for massive restores with many failures, but that would be an exceptional situation.No issues found. Ready for
make checkverification.make testpasses cleanly (no rebase needed):