-
Notifications
You must be signed in to change notification settings - Fork 19
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(recipe): refactor run-on-event recipe structure #835
Conversation
Setup map[string]any `json:"setup,omitempty" yaml:"setup,omitempty"` | ||
Type string `json:"type,omitempty" yaml:"type,omitempty"` | ||
Event string `json:"event,omitempty" yaml:"event,omitempty"` | ||
Config map[string]any `json:"config,omitempty" yaml:"config,omitempty"` |
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.
Hi @jvallesm,
I’d like to discuss an idea with you.
I’m currently refactoring the run-on-event recipe, which requires both an event-specific configuration and a connection setup. Here’s an example:
on:
<event-id>:
type: slack
event: EVENT_NEW_MESSAGE
config:
channel: xxx
setup: ${connection.my-slack}
Having both config and setup feels a bit confusing. Would it be clearer if we renamed setup to connection?
on:
<event-id>:
type: slack
event: EVENT_NEW_MESSAGE
config:
channel: xxx
connection: ${connection.my-slack}
If so, we’ll also need to update the setup
field to connection
in our component tasks for consistency. What do you think?
component:
slack-0:
type: slack
task: TASK_XXX
input:
connection: ${connection.my-slack}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #835 +/- ##
==========================================
+ Coverage 20.01% 20.12% +0.10%
==========================================
Files 354 355 +1
Lines 74750 75015 +265
==========================================
+ Hits 14963 15095 +132
- Misses 57571 57693 +122
- Partials 2216 2227 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Because
This commit