-
Notifications
You must be signed in to change notification settings - Fork 167
feat: baseline run:script, run:container and run:shell to support stdin and argv #1129
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: hiren <[email protected]>
Signed-off-by: hiren <[email protected]>
Signed-off-by: hiren <[email protected]>
Signed-off-by: hiren <[email protected]>
Signed-off-by: hiren <[email protected]>
|
ref: #1128 |
…nfiguration section Signed-off-by: hiren <[email protected]>
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.
Pull Request Overview
This PR introduces unified support for stdin and arguments parameters across run:container, run:script, and run:shell tasks. The change standardizes how data is passed to these tasks by enabling input via standard input and command-line arguments, rather than injecting data into scripts.
Key Changes:
- Changed
argumentsfromobjecttype tostring[]array for all three run task types - Added new
stdinproperty (typestring) to pass data as standard input for all three run task types - Updated examples to demonstrate the new unified syntax
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/workflow.yaml | Updated schema definitions for container, script, and shell tasks to support stdin string property and changed arguments from object to string array |
| examples/run-shell-stdin-and-arguments.yaml | New example demonstrating shell task with stdin and arguments |
| examples/run-script-with-stdin-and-arguments.yaml | New example showing script task using stdin, arguments, and environment variables |
| examples/run-script-with-arguments.yaml | Removed old example that used object-based arguments (replaced by new unified syntax) |
| examples/run-container-stdin-and-arguments.yaml | New example demonstrating container task with stdin and arguments |
| dsl-reference.md | Updated documentation for all three run task types to reflect new stdin property and array-based arguments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Can youn take a look at the CI error? |
Signed-off-by: hiren <[email protected]>
Signed-off-by: hiren <[email protected]>
|
Fix added for CI error! |
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
cdavernas
left a comment
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.
LGTM! Cheers @hirenr ❤️
|
@hirenr @cdavernas the merge is blocked because there's no verified signatures. @hirenr you must have a GPG key locally and verify your signature accordingly, then you can squash and sign your commits and force push again so I can merge it. We can manually approve the DCO to get a green, but github/CNCF rules won't allow it if you don't have a local GPG key. |
Please specify parts of this PR update:
Discussion or Issue link:
Slack Discussion Link
What this PR does:
Update the run:* syntax to have a unified structure. Introduced parameters
stdin, and updatedarguments.argumentsasstring[]passed as CLI args torun:shell/run:container/run:scriptunderlying commandPass
stdinstring via a newstringproperty to the run:shell / run:container / run:script underlying command!Additional information:
The advantages to the following are: