Skip to content

Add minItems: 1 & maxItems: 1 for subscription types#236

Merged
rartych merged 2 commits intocamaraproject:mainfrom
maxl2287:feature/add-minitems-subscription-types
Jun 25, 2024
Merged

Add minItems: 1 & maxItems: 1 for subscription types#236
rartych merged 2 commits intocamaraproject:mainfrom
maxl2287:feature/add-minitems-subscription-types

Conversation

@maxl2287
Copy link
Contributor

@maxl2287 maxl2287 commented Jun 13, 2024 •
edited
Loading

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

Limits the minimum & maximum of requested types - list to one item.

Which issue(s) this PR fixes:

Fixes #235

PedroDiez
PedroDiez previously approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@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.

LGTM

patrice-conil
patrice-conil requested changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

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

Hi @maxl2287,
To ensure that we only have one item in the array, shouldn't we also set maxItems=1

@jlurien
Copy link
Contributor

jlurien commented Jun 14, 2024

Hi @maxl2287, To ensure that we only have one item in the array, shouldn't we also set maxItems=1

That is a discussion we are having for the Geofencing API, and there consensus is that it makes sense to add the maximum as well, for APIs aligned with this version of Commonalities. In future releases, if several events per subscription are allowed, we can remove the limit.

@maxl2287 maxl2287 changed the title Add minItems: 1 for subscription types Add minItems: 1 & maxItems: 1 for subscription types Jun 17, 2024
@maxl2287
Copy link
Contributor Author

maxl2287 commented Jun 17, 2024

@jlurien @PedroDiez @patrice-conil:

I have now also set the maxItems: 1, so that we enforce one subscription-type within a request at least for this upcoming meta-release v0.4.0.

@PedroDiez
Copy link
Contributor

PedroDiez commented Jun 17, 2024

@jlurien @PedroDiez @patrice-conil:

I have now also set the maxItems: 1, so that we enforce one subscription-type within a request at least for this upcoming meta-release v0.4.0.

To me is fine if we have consensus on it @patrice-conil, @shilpa-padgaonkar, @rartych (giving LGTM in advance)

PedroDiez
PedroDiez approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@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.

LGTM

shilpa-padgaonkar
shilpa-padgaonkar approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@shilpa-padgaonkar shilpa-padgaonkar left a comment

Choose a reason for hiding this comment

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

/LGTM

patrice-conil
patrice-conil approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

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

/LGTM

bigludo7
bigludo7 approved these changes Jun 18, 2024
View reviewed changes
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

@jpengar jpengar mentioned this pull request Jun 18, 2024
Merged
@rartych rartych merged commit 3e1f133 into camaraproject:main Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PedroDiez PedroDiez PedroDiez approved these changes

@patrice-conil patrice-conil patrice-conil approved these changes

@bigludo7 bigludo7 bigludo7 approved these changes

@shilpa-padgaonkar shilpa-padgaonkar shilpa-padgaonkar approved these changes

@rartych rartych rartych approved these changes

@RubenBG7 RubenBG7 Awaiting requested review from RubenBG7

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Subscriptions-template]: Setting a minItems: 1 & maxItems: 1 for types in subscription-requests

7 participants

@maxl2287 @jlurien @PedroDiez @patrice-conil @bigludo7 @shilpa-padgaonkar @rartych