Skip to content

Semantic inconsistencies between listen and emit tasks #917

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

Closed
cdavernas opened this issue Jul 12, 2024 · 8 comments · Fixed by #963
Closed

Semantic inconsistencies between listen and emit tasks #917

cdavernas opened this issue Jul 12, 2024 · 8 comments · Fixed by #963
Labels
area: spec Changes in the Specification change: breaking A breaking change that will impact in a major version change. change: documentation Improvements or additions to documentation. It won't impact a version change.
Milestone

Comments

@cdavernas
Copy link
Member

What seems off:

The listen task allows for user to configure events to listen to in the following way:

listen:
  to:
    one:
      with: {}

Whereas the emit task does not define the with keyword:

emit:
  event: {}

What you expected to be:

The emit task to define a with keyword, use to define event properties:

emit:
  event:
    with: {}

This would allow for potential future properties such as format, and would make it more fluent, ubiquitous and consistent.

@cdavernas cdavernas added change: documentation Improvements or additions to documentation. It won't impact a version change. area: spec Changes in the Specification change: breaking A breaking change that will impact in a major version change. labels Jul 12, 2024
@cdavernas cdavernas added this to the v1.0.0-alpha3 milestone Jul 12, 2024
@cdavernas cdavernas changed the title Lexical inconsistencies between listen and emit tasks Semantic inconsistencies between listen and emit tasks Jul 12, 2024
@cdavernas
Copy link
Member Author

@ricardozanini @matthias-pichler-warrify @fjtirado What do you guys think? Should I proceed with it?

@fjtirado
Copy link
Collaborator

@fjtirado
I would actually remove with from listen ;), have you considered that approach?

@fjtirado
Copy link
Collaborator

fjtirado commented Jul 15, 2024

But yes, if you want to keep with on listen, it makes sense to add it to emit in order to make it symmetric.

@fjtirado
Copy link
Collaborator

Ok, correcting myself (I did not see the format point) yes, lets add with both for emit and listen.
Which Im not sure is the usefulness of to: maybe we can just have

listen:
   one: 
      with: 

emit: 
  event:
    with:

@JBBianchi
Copy link
Member

(Just a side note, there is a dead link in emit in the column Type, on the word "event" --> https://github.com/serverlessworkflow/specification/blob/main/dsl-reference.md#event)

@cdavernas
Copy link
Member Author

@fjtirado tbh it's mostly for aestetics and semantics that I thought of adding the with, even though, I can see new props making it there in a close future.

@cdavernas
Copy link
Member Author

@JBBianchi good catch! Would you be so kind to open a PR to fix it?

@JBBianchi
Copy link
Member

@JBBianchi good catch! Would you be so kind to open a PR to fix it?

I think this can be tackled at the same time than the current topic. That being said, I can open a PR once we agree on the direction we want to take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: spec Changes in the Specification change: breaking A breaking change that will impact in a major version change. change: documentation Improvements or additions to documentation. It won't impact a version change.
Projects
Status: Done
3 participants