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

update-design-doc-with-explicit-sub-scope-changes #177

Merged
merged 13 commits into from
May 3, 2024

Conversation

shilpa-padgaonkar
Copy link
Collaborator

@shilpa-padgaonkar shilpa-padgaonkar commented Apr 12, 2024

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

Explicit subscriptions need to be transparent about the event types that they offer.
Following changes are included:

  • Suggests that explicit subscriptions should have a separate API file
  • API name must include the keyword "subscriptions" at the end
  • Suggest how scope should be represented when actions are create (subscription) and when they are delete and read (subscription)

Which issue(s) this PR fixes:

Fixes #163

Special notes for reviewers:

Changelog input

 release-note

Additional documentation

This section can be blank.

docs

@shilpa-padgaonkar
Copy link
Collaborator Author

Changed PR to draft based on subscription related discussion. Also need to await feedback & consensus on #184

@shilpa-padgaonkar shilpa-padgaonkar marked this pull request as ready for review April 30, 2024 15:55
@shilpa-padgaonkar
Copy link
Collaborator Author

@bigludo7 and @PedroDiez : Would be great if you could kindly review the scope related changes for explicit subscriptions documented in the design guidelines and recommend changes if needed.

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.

Look good for me @shilpa-padgaonkar . I've just 3 comments for your appreciation (not blocking) and one word to correct. Thanks.

@@ -1332,6 +1354,8 @@ Note: It is perfectly valid for a CAMARA API to have several event types managed

In order to ease developer adoption, the pattern for Resource-based event subscription should be consistent for all API providing this feature.

To ensure consistency across Camara subprojects, it is necessary that explicit subscriptions are handled within separate API/s. It is recommended when possible to append the keyyword "subscriptions" at the end of the API name. For e.g. device-roaming-subscriptions.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

keyword - not keyyword

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

documentation/API-design-guidelines.md Show resolved Hide resolved
documentation/API-design-guidelines.md Show resolved Hide resolved
documentation/API-design-guidelines.md Show resolved Hide resolved
PedroDiez
PedroDiez previously approved these changes May 2, 2024
Copy link
Collaborator

@PedroDiez PedroDiez left a comment

Choose a reason for hiding this comment

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

Minor format point

LGTM in advance

documentation/API-design-guidelines.md Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
@shilpa-padgaonkar
Copy link
Collaborator Author

@PedroDiez and @bigludo7 have already approved the PR, but I would need a codeowner approval to be able to merge this. @rartych is on vacation. Could either @patrice-conil or @RubenBG7 kindly approve the PR?

@bigludo7
Copy link
Collaborator

bigludo7 commented May 3, 2024

@shilpa-padgaonkar my colleague Patrice is also enjoying well-deserved vacation :)
Hope @RubenBG7 will be able to help.

@RubenBG7
Copy link
Collaborator

RubenBG7 commented May 3, 2024

Done @shilpa-padgaonkar

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.

Subscription-Issue4: Need to change the scope pattern for explicit subscriptions
4 participants