[code-simplifier] refactor: simplify lock file reading and project config string extraction#20978
Closed
github-actions[bot] wants to merge 1 commit intomainfrom
Closed
[code-simplifier] refactor: simplify lock file reading and project config string extraction#20978github-actions[bot] wants to merge 1 commit intomainfrom
github-actions[bot] wants to merge 1 commit intomainfrom
Conversation
- Extract readLockFile() helper in run_workflow_validation.go to eliminate duplicated file reading/parsing logic between IsRunnable and getWorkflowInputs (~30 lines removed) - Use existing extractStringFromMap() helper in project_config_parsing.go to replace verbose 4-line string extraction pattern with single-line struct initialization (~27 lines removed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
@copilot lint |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors recently added workflow validation and project config parsing logic to reduce duplication and simplify string extraction.
Changes:
- Extracted a shared
readLockFile()helper to centralize lock file existence checks, reading, and YAML unmarshaling. - Simplified project config parsing by using
extractStringFromMap()forviewsandfield-definitionsstring fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/workflow/project_config_parsing.go | Replaces manual string field extraction with extractStringFromMap() and keeps required-field filtering. |
| pkg/cli/run_workflow_validation.go | Introduces readLockFile() and updates IsRunnable / getWorkflowInputs to use it. |
💡 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.
Comment on lines
+75
to
81
| field := ProjectFieldDefinition{ | ||
| Name: extractStringFromMap(fieldMap, "name", nil), | ||
| DataType: extractStringFromMap(fieldMap, "data-type", nil), | ||
| } | ||
| if dataTypeStr, ok := dataType.(string); ok { | ||
| field.DataType = dataTypeStr | ||
| if field.DataType == "" { | ||
| field.DataType = extractStringFromMap(fieldMap, "data_type", nil) | ||
| } |
Comment on lines
66
to
69
| onSection, exists := workflowYAML["on"] | ||
| if !exists { | ||
| validationLog.Printf("No 'on' section found in lock file") | ||
| // If no 'on' section, it's not runnable | ||
| return false, nil | ||
| } |
Contributor
Lint passed with 0 issues. All Go code is properly formatted and all JavaScript/JSON files pass Prettier checks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code simplification of recently merged PRs (#20951 and #20939).
Files Simplified
pkg/cli/run_workflow_validation.go— extractedreadLockFile()helper to eliminate ~30 lines of duplicated codepkg/workflow/project_config_parsing.go— replaced verbose multi-line string extraction with existingextractStringFromMap()helper (~27 lines removed)Improvements Made
1. Extracted
readLockFile()helper (run_workflow_validation.go)IsRunnableandgetWorkflowInputsshared identical logic for reading and parsing the compiled lock file:getLockFilePath+filepath.Cleanos.Statexistence check with the same error messageos.ReadFilewith#nosec G304yaml.Unmarshalwith identical error wrappingExtracted into a single
readLockFile(markdownPath string) (map[string]any, error)helper. Both callers now use it, preserving all debug logging.2. Used
extractStringFromMap()helper (project_config_parsing.go)The new
parseProjectViewsandparseProjectFieldDefinitionsfunctions (from PR #20939) used a verbose 4-line pattern to extract each string field:extractStringFromMap()already exists inpkg/workflow/config_helpers.gofor exactly this purpose. Changed to use struct initialization with single-line calls:Changes Based On
runcommand error output for missing workflow inputs #20951 — feat: improveruncommand error output for missing workflow inputsTesting
TestValidateWorkflowInputs,TestParseProjectViews,TestParseProjectFieldDefinitions, etc.)make build)go vetclean