Skip to content

Simplify data filter property names #702

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

Conversation

cdavernas
Copy link
Member

@cdavernas cdavernas commented Oct 24, 2022

Many thanks for submitting your Pull Request ❤️!

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Roadmap
  • Use Cases
  • Community
  • TCK
  • Other

Discussion or Issue link:
#672

What this PR does / why we need it:
Simplifies data filter property names, which are uselessly verbose

Special notes for reviewers:
The change applies to all concepts, except for the callbackState which exposes two filters at top level (stateDataFilter and eventDataFilter), which should anyways disapear if we go through with atomic produce/consume actions. If we keep the callbackState, we should then move the eventDataFilter down an event property, to ensure coherence accross the spec.

… instead of `stateDataFilter`, `actionDataFilter`, etc.

Signed-off-by: Charles d'Avernas <[email protected]>
@cdavernas cdavernas changed the title Simplifies data filter property names Simplify data filter property names Oct 24, 2022
@tsurdilo
Copy link
Contributor

thanks for pr! def +1 but would like if possible to wait till we deal with callback state
what we have now (before this change) is more verbose yes but consistent across all workflow states for filters

this change even tho again +1 would put spec in an inconsistent state until like you mentioned we deal with the callback state which still with your changes includes:

   "eventDataFilter": {
            "description": "Event data filter",
            "$ref": "#/definitions/eventdatafilter"
          },
          "stateDataFilter": {
            "description": "State data filter",
            "$ref": "#/definitions/statedatafilter"
          },

Are you ok with leaving this up and then add it as soon as we figure out the callback state questions?

@tsurdilo tsurdilo added the change: feature New feature or request. Impacts in a minor version change label Oct 26, 2022
@tsurdilo tsurdilo added this to the v0.9 milestone Oct 26, 2022
@cdavernas
Copy link
Member Author

@tsurdilo yeah, sure! It's just a cosmetic update anyways ;)

@tsurdilo
Copy link
Contributor

roadmap update reminder :)

Copy link
Contributor

@tsurdilo tsurdilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update roadmap

Copy link
Contributor

@tsurdilo tsurdilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs roadmap, let me know when should review again. thanks.

@cdavernas
Copy link
Member Author

Will do asap, thanks!

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

github-actions bot commented Mar 6, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@cdavernas
Copy link
Member Author

@tsurdilo The roadmap has finally been updated ;) Sorry for the very long delay, had forgot about the PR

@cdavernas cdavernas linked an issue May 30, 2023 that may be closed by this pull request
@cdavernas cdavernas self-assigned this May 30, 2023
@ricardozanini
Copy link
Member

@tsurdilo can you please take a look?

@cdavernas cdavernas added area: spec Changes in the Specification impacts SDK 🔧 labels May 30, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@cdavernas cdavernas requested a review from tsurdilo February 16, 2024 08:36
@cdavernas cdavernas dismissed tsurdilo’s stale review February 16, 2024 08:37

Bug: requested changes have all been addressed but is still marked

@cdavernas
Copy link
Member Author

Have to refactor that one too :(

Not surprisingly, became stale after like 2 years of sitting here

@ricardozanini
Copy link
Member

Closed as part of #843

@cdavernas cdavernas deleted the data-filter-naming branch May 17, 2024 14:38
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: feature New feature or request. Impacts in a minor version change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename data filter properties
3 participants