-
Notifications
You must be signed in to change notification settings - Fork 432
Refactor: Extract scattered helper functions into dedicated files #3601
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
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,57 +1,4 @@ | ||
| package workflow | ||
|
|
||
| import ( | ||
| "github.com/githubnext/gh-aw/pkg/logger" | ||
| ) | ||
|
|
||
| var configLog = logger.New("workflow:config") | ||
|
|
||
| // parseLabelsFromConfig extracts and validates labels from a config map | ||
| // Returns a slice of label strings, or nil if labels is not present or invalid | ||
| func parseLabelsFromConfig(configMap map[string]any) []string { | ||
| if labels, exists := configMap["labels"]; exists { | ||
| configLog.Print("Parsing labels from config") | ||
| if labelsArray, ok := labels.([]any); ok { | ||
| var labelStrings []string | ||
| for _, label := range labelsArray { | ||
| if labelStr, ok := label.(string); ok { | ||
| labelStrings = append(labelStrings, labelStr) | ||
| } | ||
| } | ||
| // Return the slice even if empty (to distinguish from not provided) | ||
| if labelStrings == nil { | ||
| configLog.Print("No valid label strings found, returning empty array") | ||
| return []string{} | ||
| } | ||
| configLog.Printf("Parsed %d labels from config", len(labelStrings)) | ||
| return labelStrings | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // parseStringFromConfig is a generic helper that extracts and validates a string value from a config map | ||
| // Returns the string value, or empty string if not present or invalid | ||
| func parseStringFromConfig(configMap map[string]any, key string) string { | ||
| if value, exists := configMap[key]; exists { | ||
| if valueStr, ok := value.(string); ok { | ||
| configLog.Printf("Parsed %s from config: %s", key, valueStr) | ||
| return valueStr | ||
| } | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // parseTitlePrefixFromConfig extracts and validates title-prefix from a config map | ||
| // Returns the title prefix string, or empty string if not present or invalid | ||
| func parseTitlePrefixFromConfig(configMap map[string]any) string { | ||
| return parseStringFromConfig(configMap, "title-prefix") | ||
| } | ||
|
|
||
| // parseTargetRepoFromConfig extracts the target-repo value from a config map. | ||
| // Returns the target repository slug as a string, or empty string if not present or invalid. | ||
| // This function does not perform any special handling or validation for wildcard values ("*"); | ||
| // callers are responsible for validating the returned value as needed. | ||
| func parseTargetRepoFromConfig(configMap map[string]any) string { | ||
| return parseStringFromConfig(configMap, "target-repo") | ||
| } | ||
| // Configuration parsing helpers have been moved to config_helpers.go | ||
| // This file is kept as a placeholder for any future config-related functionality | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package workflow | ||
|
|
||
| import ( | ||
| "github.com/githubnext/gh-aw/pkg/logger" | ||
| ) | ||
|
|
||
| var configHelpersLog = logger.New("workflow:config_helpers") | ||
|
Comment on lines
+1
to
+7
|
||
|
|
||
| // parseLabelsFromConfig extracts and validates labels from a config map | ||
| // Returns a slice of label strings, or nil if labels is not present or invalid | ||
| func parseLabelsFromConfig(configMap map[string]any) []string { | ||
| if labels, exists := configMap["labels"]; exists { | ||
| configHelpersLog.Print("Parsing labels from config") | ||
| if labelsArray, ok := labels.([]any); ok { | ||
| var labelStrings []string | ||
| for _, label := range labelsArray { | ||
| if labelStr, ok := label.(string); ok { | ||
| labelStrings = append(labelStrings, labelStr) | ||
| } | ||
| } | ||
| // Return the slice even if empty (to distinguish from not provided) | ||
| if labelStrings == nil { | ||
| configHelpersLog.Print("No valid label strings found, returning empty array") | ||
| return []string{} | ||
| } | ||
| configHelpersLog.Printf("Parsed %d labels from config", len(labelStrings)) | ||
| return labelStrings | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // parseStringFromConfig is a generic helper that extracts and validates a string value from a config map | ||
| // Returns the string value, or empty string if not present or invalid | ||
| func parseStringFromConfig(configMap map[string]any, key string) string { | ||
| if value, exists := configMap[key]; exists { | ||
| if valueStr, ok := value.(string); ok { | ||
| configHelpersLog.Printf("Parsed %s from config: %s", key, valueStr) | ||
| return valueStr | ||
| } | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // parseTitlePrefixFromConfig extracts and validates title-prefix from a config map | ||
| // Returns the title prefix string, or empty string if not present or invalid | ||
| func parseTitlePrefixFromConfig(configMap map[string]any) string { | ||
| return parseStringFromConfig(configMap, "title-prefix") | ||
| } | ||
|
|
||
| // parseTargetRepoFromConfig extracts the target-repo value from a config map. | ||
| // Returns the target repository slug as a string, or empty string if not present or invalid. | ||
| // This function does not perform any special handling or validation for wildcard values ("*"); | ||
| // callers are responsible for validating the returned value as needed. | ||
| func parseTargetRepoFromConfig(configMap map[string]any) string { | ||
| return parseStringFromConfig(configMap, "target-repo") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package workflow | ||
|
|
||
|
Comment on lines
+1
to
+2
|
||
| // extractStringValue extracts a string value from the frontmatter map | ||
| func extractStringValue(frontmatter map[string]any, key string) string { | ||
| value, exists := frontmatter[key] | ||
| if !exists { | ||
| return "" | ||
| } | ||
|
|
||
| if strValue, ok := value.(string); ok { | ||
| return strValue | ||
| } | ||
|
|
||
| return "" | ||
| } | ||
|
|
||
| // parseIntValue safely parses various numeric types to int | ||
| // This is a common utility used across multiple parsing functions | ||
| func parseIntValue(value any) (int, bool) { | ||
| switch v := value.(type) { | ||
| case int: | ||
| return v, true | ||
| case int64: | ||
| return int(v), true | ||
| case uint64: | ||
| return int(v), true | ||
| case float64: | ||
| return int(v), true | ||
| default: | ||
| return 0, false | ||
| } | ||
| } | ||
|
|
||
| // filterMapKeys creates a new map excluding the specified keys | ||
| func filterMapKeys(original map[string]any, excludeKeys ...string) map[string]any { | ||
| excludeSet := make(map[string]bool) | ||
| for _, key := range excludeKeys { | ||
| excludeSet[key] = true | ||
| } | ||
|
|
||
| result := make(map[string]any) | ||
| for key, value := range original { | ||
| if !excludeSet[key] { | ||
| result[key] = value | ||
| } | ||
| } | ||
| return result | ||
| } | ||
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.
This file should be removed rather than kept as an empty placeholder. According to the project's coding guidelines on file organization, empty placeholder files are considered an anti-pattern. Since all the configuration parsing helpers have been moved to
config_helpers.go, this file no longer serves a purpose and keeping it adds unnecessary clutter to the codebase.If configuration-related functionality needs to be added in the future, a new file can be created at that time with actual content.
See below for a potential fix: