You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Architecture review found unused types and config fields that expand the public API surface without implementation backing.
Finding 1: Unused option types
File:types/options.go
CreateOptions, GetOptions, DeleteOptions, and UpdateOptions are empty structs never accepted by any method. They mislead users into thinking they can configure something.
Fix: Remove from types/options.go and v1/options.go. Add back when a method actually accepts them.
Finding 2: Unused Config fields
File:types/config.go:13-18
Config.Timeout, Config.RetryPolicy, and Config.Logger are marked "reserved for future use" but silently ignored by NewClient. Users setting Timeout: 5*time.Second get no timeout behavior.
Fix: Remove Timeout, RetryPolicy, Logger from Config. Remove RetryPolicy struct from types/types.go. Add back when implemented.
Finding 3: Unused WatchOptions fields
File:types/options.go:29-34
WatchOptions.TimeoutSeconds and WatchOptions.LabelSelector are reserved but never read by Watch.
Fix: Remove the reserved fields, leaving only StopOnTerminal.
Deferred to the next upstream contribution PR. These types exist in the upstream NVIDIA/OpenShell repo (merged in Drop A) with // reserved for future use comments. Removing them here would just re-introduce them on the next proto sync. Added to brainstorm/030-upstream-review-findings.md for tracking.
Activity
rhuss commentedon Aug 5, 2026
Those are will be filled by the second drop.
rhuss commentedon Aug 8, 2026
Deferred to the next upstream contribution PR. These types exist in the upstream NVIDIA/OpenShell repo (merged in Drop A) with
// reserved for future usecomments. Removing them here would just re-introduce them on the next proto sync. Added to brainstorm/030-upstream-review-findings.md for tracking.docs: add YAGNI removal to upstream review findings (#51)