Conversation
There was a problem hiding this comment.
Pull request overview
Removes unused String() / IsValid() helper methods from several semantic type aliases in pkg/constants, along with the corresponding unit tests, as flagged by the deadcode tool.
Changes:
- Deleted
String()/IsValid()methods for multiple semantic type aliases (e.g.,LineLength,FeatureFlag,URL,ModelName,WorkflowID,EngineName,MCPServerID). - Removed unit tests that exercised the deleted helper methods.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/constants/constants.go | Removes dead helper methods and the now-unused fmt import from constants semantic types. |
| pkg/constants/constants_test.go | Deletes tests that depended on the removed helper methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type LineLength int | ||
|
|
There was a problem hiding this comment.
scratchpad/go-type-patterns.md includes code examples for LineLength, WorkflowID, and EngineName that still reference the deleted String() / IsValid() methods (see e.g. scratchpad/go-type-patterns.md around the “LineLength Type” / “WorkflowID Type” sections). Since this PR removes those methods, the documentation examples should be updated to avoid showing non-compiling code.
| type LineLength int | ||
|
|
There was a problem hiding this comment.
Removing the exported String()/IsValid() methods from semantic types (e.g., LineLength/WorkflowID/EngineName/etc.) is a breaking change for any downstream Go code importing github.com/github/gh-aw/pkg/constants, even if the methods are unused داخل this repo. If this package is considered part of the public Go API, consider either (a) keeping these methods with // Deprecated: docs for a deprecation window, or (b) documenting the breaking change via the project’s changeset/CHANGELOG process.
Removes dead String() and IsValid() methods from semantic type aliases in pkg/constants/constants.go — flagged by deadcode tool.
Deleted: LineLength.String, LineLength.IsValid, FeatureFlag.String, FeatureFlag.IsValid, URL.String, URL.IsValid, ModelName.String, ModelName.IsValid, WorkflowID.String, WorkflowID.IsValid, EngineName.String, EngineName.IsValid, MCPServerID.IsValid — 13 functions total.