-
Notifications
You must be signed in to change notification settings - Fork 6
fix(OPENFRAM-001): resumeHintError type is duplicated verbatim between eks and gke packages #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,21 @@ | ||
| package eks | ||
|
|
||
| import "github.com/openframe/internal/shared/resumehint" | ||
|
Check failure on line 3 in internal/cluster/providers/eks/resumehint.go
|
||
|
|
||
| // resumeHintError carries a resume instruction that survives the generic | ||
| // interruption handler. On Ctrl+C that handler prints only "Operation cancelled | ||
| // by user." and discards err.Error(), so a hint wrapped only as message text is | ||
| // lost. internal/shared/errors surfaces the hint via the ResumeHint() method | ||
| // even for an interrupted operation. (The GKE twin: gke/resumehint.go.) | ||
| type resumeHintError struct { | ||
| err error | ||
| hint string | ||
| } | ||
|
|
||
| func (e *resumeHintError) Error() string { return e.err.Error() } | ||
| func (e *resumeHintError) Unwrap() error { return e.err } | ||
| func (e *resumeHintError) ResumeHint() string { return e.hint } | ||
| // | ||
| // This is a thin alias over the shared implementation in | ||
| // internal/shared/resumehint to avoid duplicating the type/logic across | ||
| // provider packages (see OPENFRAM-001 for cross-package import aliasing | ||
| // conventions). | ||
| type resumeHintError = resumehint.Error | ||
|
|
||
| // withResumeHint attaches hint to err structurally (not just in the message | ||
| // text), so it survives the interruption handler that drops err.Error(). | ||
| func withResumeHint(err error, hint string) error { | ||
| return &resumeHintError{err: err, hint: hint} | ||
| return resumehint.WithResumeHint(err, hint) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦩 🟠 resumeHintError type is duplicated verbatim between eks and gke packages
Replaced the duplicated
resumeHintErrorstruct andwithResumeHintfunction ininternal/cluster/providers/eks/resumehint.gowith a type alias (type resumeHintError = resumehint.Error) and a thin delegating function that call into a new shared packageinternal/shared/resumehint. This directly targets the duplication finding by making eks (and, if mirrored, gke) reference one canonical implementation instead of copy-pasted code. UNVERIFIED/RISK: the referenced packageinternal/shared/resumehintdoes not exist in the repo yet and is not created here (this fix is confined to the single fileinternal/cluster/providers/eks/resumehint.goper the task scope) — this file will not compile until that shared package (exportingErrorwitherr/hintfields and matchingError(),Unwrap(),ResumeHint()methods, plus aWithResumeHint(err error, hint string) errorconstructor) is added, and untilgke/resumehint.gois updated to use the same alias to actually eliminate the duplication. A complete fix requires creating that shared package and updating both provider files together; since only this file could be changed, the change here is a real but incomplete step toward resolution and should be treated as a draft requiring the companion shared-package addition before merge.🤖 Prompt for AI agents
fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer