Skip to content
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

Align event notification with CloudEvents spec #224

Merged

Conversation

jlurien
Copy link
Collaborator

@jlurien jlurien commented Oct 6, 2023

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

Align QoD event notification with recently approved guidelines adhering to CloudEvents specification

Which issue(s) this PR fixes:

Fixes #221

Changelog input

- Notification event changed to comply with CloudEvents specification

@hdamker
Copy link
Collaborator

hdamker commented Nov 3, 2023

The last commit is about a month ago here. Does it mean that we are ready here? ... Guess that we need your reviews @jlurien @eric-murray @RandyLevensalor @akoshunyadi

@jlurien
Copy link
Collaborator Author

jlurien commented Nov 3, 2023

The last commit is about a month ago here. Does it mean that we are ready here? ... Guess that we need your reviews @jlurien @eric-murray @RandyLevensalor @akoshunyadi

PR is mine so I cannot review it myself, but yes, we need this to be reviewed, it is an important change. I would add @bigludo7 as reviewer here as he is in charge of this same tasks in other subprojects.

@hdamker hdamker requested a review from bigludo7 November 3, 2023 12:59
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

@jlurien Looking for your guidance about presence or not of data attribute in CloudEvent class.

code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-api.yaml Show resolved Hide resolved
- Added data to CloudEvents base class
- Notification example moved to callback
jlurien and others added 5 commits November 17, 2023 12:36
- Move QoS status changed event example to components, to be aligned with template in Commonalities
added eol at the end of the file to suit linting
removing trailing spaces
Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

LGTM

@hdamker
Copy link
Collaborator

hdamker commented Nov 24, 2023

@bigludo7 @eric-murray @akoshunyadi @RandyLevensalor @akoshunyadi : there are no new comments since last week. Are we good to go to merge today?

Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

LGTM

@jlurien
Copy link
Collaborator Author

jlurien commented Nov 27, 2023

@hdamker I think this is ready to be merged. Thanks for all comments and feedback

@hdamker hdamker merged commit 1ebca6f into camaraproject:main Nov 27, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change event structure and notification according to Design Guidelines changes
6 participants