-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/run if #23
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
Feat/run if #23
Conversation
Reviewer's Guide by SourceryThis PR implements conditional command execution in tasks using a new 'if' field and refactors command parsing logic. The changes introduce a new ParsedCommandJson struct to handle evaluated conditions and improve error handling for command parsing. Sequence diagram for evalGoTemplateCondition functionsequenceDiagram
participant Caller
participant evalGoTemplateCondition
participant template
participant bytes.Buffer
Caller->>evalGoTemplateCondition: Call with tpl
evalGoTemplateCondition->>template: Create new template
evalGoTemplateCondition->>template: Parse template expression
alt Parsing error
evalGoTemplateCondition-->>Caller: Return TaskRequirementIncorrect error
end
evalGoTemplateCondition->>bytes.Buffer: Create new buffer
evalGoTemplateCondition->>template: Execute template
alt Execution error
evalGoTemplateCondition-->>Caller: Return TaskRequirementIncorrect error
end
alt Template evaluates to "true"
evalGoTemplateCondition-->>Caller: Return true
else
evalGoTemplateCondition-->>Caller: Return TaskRequirementFailed error
end
Sequence diagram for runTask function with conditional executionsequenceDiagram
participant runTask
participant logger
participant command
loop for each command in pt.Commands
runTask->>logger: Debug "running command task"
alt command.If is not nil and false
runTask->>logger: Debug "skipping execution for failed `if`"
runTask-->>command: Continue
end
alt command.Run is not empty
runTask->>runTask: Call recursively with command.Run
alt Error occurs
runTask-->>caller: Return error
end
end
end
Updated class diagram for ParsedTask and CommandJsonclassDiagram
class ParsedTask {
+[]string Shell
+string WorkingDir
+map<string, string> Env
+bool Interactive
+[]ParsedCommandJson Commands
}
class CommandJson {
+string Command
+string Run
+string Env
+*string If
}
class ParsedCommandJson {
+string Command
+string Run
+string Env
+*bool If
}
ParsedTask --> ParsedCommandJson
CommandJson --> ParsedCommandJson
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @nxtcoder17 - I've reviewed your changes - here's some feedback:
Overall Comments:
- There is commented out error handling code in parseCommand() that should either be uncommented or removed if not needed
- The validation check for run targets is commented out - this should probably be restored to prevent runtime errors from invalid task references
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| pcj.Command = cj.Command | ||
|
|
||
| if cj.If != nil { | ||
| ok, _ := evalGoTemplateCondition(*cj.If) |
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.
issue (bug_risk): Error from evalGoTemplateCondition should not be ignored
Silently ignoring template evaluation errors could mask serious problems. Consider handling or propagating the error.
| } | ||
|
|
||
| return &cj, nil | ||
| // if _, ok := rf.Tasks[cj.Run]; !ok { |
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.
issue (bug_risk): Run target existence check should be retained
Removing this check could lead to runtime failures when trying to execute non-existent tasks.
Resolves #24
Summary by Sourcery
Add support for conditional task execution using Go template expressions, enabling tasks to be executed only when certain conditions are met. Refactor the task parsing logic to improve code maintainability.
New Features:
Enhancements: