Skip to content

chore: remove dead String/IsValid methods from constants (batch 9) #18818

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.

Sign up for GitHub

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

Merged
dsyme merged 1 commit into from
Feb 28, 2026
+0 −168
Merged

chore: remove dead String/IsValid methods from constants (batch 9) #18818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 0 additions & 66 deletions pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package constants

import (
"fmt"
"path/filepath"
"time"
)
Expand Down Expand Up @@ -34,16 +33,6 @@ const CLIExtensionPrefix CommandPrefix = "gh aw"
// }
type LineLength int

Comment on lines 34 to 35
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 34 to 35
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
// String returns the string representation of the line length
func (l LineLength) String() string {
return fmt.Sprintf("%d", l)
}

// IsValid returns true if the line length is positive
func (l LineLength) IsValid() bool {
return l > 0
}

// Version represents a software version string.
// This semantic type distinguishes version strings from arbitrary strings,
// enabling future validation logic (e.g., semver parsing) and making
Expand Down Expand Up @@ -75,16 +64,6 @@ func (v Version) IsValid() bool {
// func IsFeatureEnabled(flag FeatureFlag) bool { ... }
type FeatureFlag string

// String returns the string representation of the feature flag
func (f FeatureFlag) String() string {
return string(f)
}

// IsValid returns true if the feature flag is non-empty
func (f FeatureFlag) IsValid() bool {
return len(f) > 0
}

// URL represents a URL string.
// This semantic type distinguishes URLs from arbitrary strings,
// making URL parameters explicit and enabling future validation logic.
Expand All @@ -95,16 +74,6 @@ func (f FeatureFlag) IsValid() bool {
// func FetchFromRegistry(url URL) error { ... }
type URL string

// String returns the string representation of the URL
func (u URL) String() string {
return string(u)
}

// IsValid returns true if the URL is non-empty
func (u URL) IsValid() bool {
return len(u) > 0
}

// ModelName represents an AI model name identifier.
// This semantic type distinguishes model names from arbitrary strings,
// making model selection explicit in function signatures.
Expand All @@ -115,16 +84,6 @@ func (u URL) IsValid() bool {
// func ExecuteWithModel(model ModelName) error { ... }
type ModelName string

// String returns the string representation of the model name
func (m ModelName) String() string {
return string(m)
}

// IsValid returns true if the model name is non-empty
func (m ModelName) IsValid() bool {
return len(m) > 0
}

// JobName represents a GitHub Actions job identifier.
// This semantic type distinguishes job names from arbitrary strings,
// preventing mixing of job identifiers with other string types.
Expand Down Expand Up @@ -195,16 +154,6 @@ func (c CommandPrefix) IsValid() bool {
// func CompileWorkflow(id WorkflowID) error { ... }
type WorkflowID string

// String returns the string representation of the workflow ID
func (w WorkflowID) String() string {
return string(w)
}

// IsValid returns true if the workflow ID is non-empty
func (w WorkflowID) IsValid() bool {
return len(w) > 0
}

// EngineName represents an AI engine name identifier (copilot, claude, codex, custom).
// This semantic type distinguishes engine names from arbitrary strings,
// making engine selection explicit and type-safe.
Expand All @@ -215,16 +164,6 @@ func (w WorkflowID) IsValid() bool {
// func SetEngine(engine EngineName) error { ... }
type EngineName string

// String returns the string representation of the engine name
func (e EngineName) String() string {
return string(e)
}

// IsValid returns true if the engine name is non-empty
func (e EngineName) IsValid() bool {
return len(e) > 0
}

// DocURL represents a documentation URL for error messages and help text.
// This semantic type distinguishes documentation URLs from arbitrary URLs,
// making documentation references explicit and centralized for easier maintenance.
Expand Down Expand Up @@ -666,11 +605,6 @@ func (m MCPServerID) String() string {
return string(m)
}

// IsValid returns true if the MCP server ID is non-empty
func (m MCPServerID) IsValid() bool {
return len(m) > 0
}

// SafeOutputsMCPServerID is the identifier for the safe-outputs MCP server
const SafeOutputsMCPServerID MCPServerID = "safeoutputs"

Expand Down
102 changes: 0 additions & 102 deletions pkg/constants/constants_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,6 @@ func TestSemanticTypeAliases(t *testing.T) {
if string(testWorkflow) != "ci-doctor" {
t.Errorf("WorkflowID conversion failed: got %q, want %q", testWorkflow, "ci-doctor")
}

// Test that WorkflowID can hold typical workflow identifiers
workflows := []WorkflowID{"ci-doctor", "deploy-prod", "test-workflow"}
for i, wf := range workflows {
if !wf.IsValid() {
t.Errorf("WorkflowID[%d] should be valid: %q", i, wf)
}
}
})

// Test EngineName type
Expand Down Expand Up @@ -528,26 +520,6 @@ func TestTypeSafetyBetweenSemanticTypes(t *testing.T) {

// TestHelperMethods tests the helper methods on semantic types
func TestHelperMethods(t *testing.T) {
t.Run("LineLength", func(t *testing.T) {
length := LineLength(120)
if length.String() != "120" {
t.Errorf("LineLength.String() = %q, want %q", length.String(), "120")
}
if !length.IsValid() {
t.Error("LineLength.IsValid() = false, want true for positive value")
}

invalidLength := LineLength(0)
if invalidLength.IsValid() {
t.Error("LineLength.IsValid() = true, want false for zero value")
}

negativeLength := LineLength(-1)
if negativeLength.IsValid() {
t.Error("LineLength.IsValid() = true, want false for negative value")
}
})

t.Run("Version", func(t *testing.T) {
version := Version("1.0.0")
if version.String() != "1.0.0" {
Expand All @@ -563,51 +535,6 @@ func TestHelperMethods(t *testing.T) {
}
})

t.Run("FeatureFlag", func(t *testing.T) {
flag := FeatureFlag("test-flag")
if flag.String() != "test-flag" {
t.Errorf("FeatureFlag.String() = %q, want %q", flag.String(), "test-flag")
}
if !flag.IsValid() {
t.Error("FeatureFlag.IsValid() = false, want true for non-empty value")
}

emptyFlag := FeatureFlag("")
if emptyFlag.IsValid() {
t.Error("FeatureFlag.IsValid() = true, want false for empty value")
}
})

t.Run("URL", func(t *testing.T) {
url := URL("https://example.com")
if url.String() != "https://example.com" {
t.Errorf("URL.String() = %q, want %q", url.String(), "https://example.com")
}
if !url.IsValid() {
t.Error("URL.IsValid() = false, want true for non-empty value")
}

emptyURL := URL("")
if emptyURL.IsValid() {
t.Error("URL.IsValid() = true, want false for empty value")
}
})

t.Run("ModelName", func(t *testing.T) {
model := ModelName("gpt-5-mini")
if model.String() != "gpt-5-mini" {
t.Errorf("ModelName.String() = %q, want %q", model.String(), "gpt-5-mini")
}
if !model.IsValid() {
t.Error("ModelName.IsValid() = false, want true for non-empty value")
}

emptyModel := ModelName("")
if emptyModel.IsValid() {
t.Error("ModelName.IsValid() = true, want false for empty value")
}
})

t.Run("JobName", func(t *testing.T) {
job := JobName("agent")
if job.String() != "agent" {
Expand Down Expand Up @@ -653,35 +580,6 @@ func TestHelperMethods(t *testing.T) {
}
})

t.Run("WorkflowID", func(t *testing.T) {
workflow := WorkflowID("ci-doctor")
if workflow.String() != "ci-doctor" {
t.Errorf("WorkflowID.String() = %q, want %q", workflow.String(), "ci-doctor")
}
if !workflow.IsValid() {
t.Error("WorkflowID.IsValid() = false, want true for non-empty value")
}

emptyWorkflow := WorkflowID("")
if emptyWorkflow.IsValid() {
t.Error("WorkflowID.IsValid() = true, want false for empty value")
}
})

t.Run("EngineName", func(t *testing.T) {
engine := EngineName("copilot")
if engine.String() != "copilot" {
t.Errorf("EngineName.String() = %q, want %q", engine.String(), "copilot")
}
if !engine.IsValid() {
t.Error("EngineName.IsValid() = false, want true for non-empty value")
}

emptyEngine := EngineName("")
if emptyEngine.IsValid() {
t.Error("EngineName.IsValid() = true, want false for empty value")
}
})
}

func TestGetAllEngineSecretNames(t *testing.T) {
Expand Down
Loading