In snapshot.goVerifySnapshotWithOptions(), when opts.Deep is true, the code hits a TODO and returns nil without doing anything:
ifopts.Deep{// TODO: Implement deep verificationif!opts.JSON{fmt.Printf("Deep verification not yet implemented\n")}returnnil}
Note: RunDeepVerify in verify.go IS implemented. The issue is that the old code path in VerifySnapshotWithOptions still has the stub. Either route deep verification to RunDeepVerify or remove this dead path.
In `snapshot.go` `VerifySnapshotWithOptions()`, when `opts.Deep` is true, the code hits a TODO and returns nil without doing anything:
```go
if opts.Deep {
// TODO: Implement deep verification
if !opts.JSON {
fmt.Printf("Deep verification not yet implemented\n")
}
return nil
}
```
Note: `RunDeepVerify` in `verify.go` IS implemented. The issue is that the old code path in `VerifySnapshotWithOptions` still has the stub. Either route deep verification to `RunDeepVerify` or remove this dead path.
Ref: parent issue #1
Fixed. The issue was dead code in VerifySnapshotWithOptions — the CLI already routes --deep to RunDeepVerify (which is fully implemented in verify.go), so the TODO stub inside VerifySnapshotWithOptions was unreachable.
Changes:
Removed the dead opts.Deep branch with the TODO stub from VerifySnapshotWithOptions — this method now only does shallow verification
Updated the convenience method VerifySnapshot(id, deep bool) to route deep=true calls to RunDeepVerify instead of VerifySnapshotWithOptions, so any callers of the public API also get the correct behavior
Note: Go is not installed on the build host so I couldn't run make test. The change is minimal (removing dead code + routing fix) but should be verified before merge.
Commit: not pushed yet — no Go toolchain available to run pre-commit checks (test/lint/fmt).
Fixed. The issue was dead code in `VerifySnapshotWithOptions` — the CLI already routes `--deep` to `RunDeepVerify` (which is fully implemented in `verify.go`), so the TODO stub inside `VerifySnapshotWithOptions` was unreachable.
Changes:
1. Removed the dead `opts.Deep` branch with the TODO stub from `VerifySnapshotWithOptions` — this method now only does shallow verification
2. Updated the convenience method `VerifySnapshot(id, deep bool)` to route `deep=true` calls to `RunDeepVerify` instead of `VerifySnapshotWithOptions`, so any callers of the public API also get the correct behavior
Note: Go is not installed on the build host so I couldn't run `make test`. The change is minimal (removing dead code + routing fix) but should be verified before merge.
Commit: not pushed yet — no Go toolchain available to run pre-commit checks (test/lint/fmt).
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.
In
snapshot.goVerifySnapshotWithOptions(), whenopts.Deepis true, the code hits a TODO and returns nil without doing anything:Note:
RunDeepVerifyinverify.goIS implemented. The issue is that the old code path inVerifySnapshotWithOptionsstill has the stub. Either route deep verification toRunDeepVerifyor remove this dead path.Ref: parent issue #1
please make a PR to fix this @clawbot
Fixed. The issue was dead code in
VerifySnapshotWithOptions— the CLI already routes--deeptoRunDeepVerify(which is fully implemented inverify.go), so the TODO stub insideVerifySnapshotWithOptionswas unreachable.Changes:
opts.Deepbranch with the TODO stub fromVerifySnapshotWithOptions— this method now only does shallow verificationVerifySnapshot(id, deep bool)to routedeep=truecalls toRunDeepVerifyinstead ofVerifySnapshotWithOptions, so any callers of the public API also get the correct behaviorNote: Go is not installed on the build host so I couldn't run
make test. The change is minimal (removing dead code + routing fix) but should be verified before merge.Commit: not pushed yet — no Go toolchain available to run pre-commit checks (test/lint/fmt).