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

split /subscription-endpoints into seperate APIs #161

Merged

Conversation

maxl2287
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature
  • cleanup

What this PR does / why we need it:

Splits up the subscription-endpoints for reachability and roaming status into seperate YAML-files.

Which issue(s) this PR fixes:

Fixes #125

@bigludo7
Copy link
Collaborator

Thanks @maxl2287 for the effort.
I guess we have to align with the new model for subscription.
Do we handle this in this PR or in a separate one?
cc: @akoshunyadi

@maxl2287
Copy link
Contributor Author

@bigludo7 tbh I would handle this in a seperate PR 😄

Do you both agree?

@bigludo7
Copy link
Collaborator

@bigludo7 tbh I would handle this in a seperate PR 😄

Do you both agree?

Works for me.
It means that we have to merge this one quickly in order to no block you to move forward with this newt PR - Correct?

@akoshunyadi
Copy link
Collaborator

ok for me. Yes, then then we need an additional PR for the subscriptions.

bigludo7
bigludo7 previously approved these changes Jun 11, 2024
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.

OK for me in order to move forward to the new model.

@maxl2287
Copy link
Contributor Author

@sachinvodafone thanks for the findings!

bigludo7
bigludo7 previously approved these changes Jun 11, 2024
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.

OK for me

@maxl2287
Copy link
Contributor Author

@bigludo7 can we label this as v0.6.0 ?
(I am not allowed to 🥲 )

@bigludo7 bigludo7 added the v0.6.0 Planned for release v0.6.0 label Jun 11, 2024
Copy link
Collaborator

@akoshunyadi akoshunyadi left a comment

Choose a reason for hiding this comment

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

Just some typos... the big rework based on the new template will come anyway..

bigludo7
bigludo7 previously approved these changes Jun 12, 2024
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

@maxl2287
Copy link
Contributor Author

@bigludo7 I was needed to update the file-name to device-reachability-status-subscriptions.yaml

Please review again :)

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.

We need to align with PR213 outcomes for the error linked to the device identifier but in order to not manage too complex PR I will prefer to merge this one and then raise another issue/PR to manage 213.
@maxl2287 @akoshunyadi @sachinvodafone WDYT?

- $ref: '#/components/parameters/x-correlator'
security:
- openId:
- reachability-status:subscriptions:create
Copy link
Collaborator

@akoshunyadi akoshunyadi Jun 17, 2024

Choose a reason for hiding this comment

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

Actually the scope should be api-name:[resource:]action, so in this case device-reachabilty-status-subscriptions:create
(applies to other scopes in this PR too)

@akoshunyadi
Copy link
Collaborator

@maxl2287 we can also merge this PR as it is since #162 needs to be done anyway, which will rework it, right?

@akoshunyadi akoshunyadi merged commit 9be3cba into camaraproject:main Jun 18, 2024
1 check passed
@maxl2287 maxl2287 deleted the feature/split-subscription-apis branch June 18, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v0.6.0 Planned for release v0.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate endpoints into different APIs
4 participants