explicitly return a string for String method instead of panic#112
explicitly return a string for String method instead of panic#112acornett21 merged 1 commit intooperator-framework:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the pkg/imagename.ImageName String() implementation to avoid panicking when string formatting fails, aligning fmt.Stringer behavior with the expectation that String() should be safe for callers.
Changes:
- Replace
panic(err)in(*ImageName).String()with a sentinel return value ("<invalid>") on error. - Add an explanatory comment documenting why
String()must not panic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR updates ImageName.String() (a fmt.Stringer implementation) to avoid panicking on invalid image names by returning "<invalid>", and adds test coverage for invalid ImageName values.
Changes:
- Change
ImageName.String()to return"<invalid>"when formatting fails, instead of panicking. - Add Ginkgo table tests asserting
String()returns"<invalid>"for invalidImageNamestruct states.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/imagename/imagename.go | Replaces panic-on-error behavior in String() with a sentinel string return. |
| pkg/imagename/imagename_test.go | Adds table-driven tests for invalid ImageName.String() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR updates the ImageName String() implementation to avoid panicking (even for invalid or nil receivers) and adds tests ensuring invalid image names produce a stable, explicit placeholder string.
Changes:
- Updated
(*ImageName).String()to return"<invalid>"instead of panicking on invalid state/errors. - Added Ginkgo table-driven tests covering nil/invalid
ImageNamecases forString().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/imagename/imagename.go | Makes String() nil-safe and non-panicking by returning "<invalid>" on failure. |
| pkg/imagename/imagename_test.go | Adds coverage to ensure String() returns "<invalid>" for several invalid ImageName shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR updates the ImageName String() implementation to avoid panicking on invalid values (including a nil receiver) and instead return a clear sentinel string (<invalid>), aligning with Stringer expectations.
Changes:
- Add
<invalid>sentinel constant for representing invalidImageNamestringification results. - Update
(*ImageName).String()to return<invalid>instead of panicking (and to handle nil receivers safely). - Add table-driven tests covering invalid
ImageNamecases (including nil) to ensureString()does not panic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/imagename/imagename.go | Introduces <invalid> sentinel and makes String() return it on nil/invalid state instead of panicking. |
| pkg/imagename/imagename_test.go | Adds Ginkgo table tests verifying String() returns <invalid> for invalid/nil ImageName. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Signed-off-by: Adam D. Cornett <adc@redhat.com>
<invalid>so it's clear to the caller that something is wrong.