Skip to content

fix: add missing "subscriptionId" in "RoamingStatus"-data for CloudEvent#109

Merged
akoshunyadi merged 3 commits intocamaraproject:mainfrom
maxl2287:fix/update-events
Mar 13, 2024
Merged

fix: add missing "subscriptionId" in "RoamingStatus"-data for CloudEvent#109
akoshunyadi merged 3 commits intocamaraproject:mainfrom
maxl2287:fix/update-events

Conversation

@maxl2287
Copy link
Contributor

@maxl2287 maxl2287 commented Feb 7, 2024

What type of PR is this?

Add one of the following kinds:

  • correction

What this PR does / why we need it:

In the examples of callback CloudEvents for roaming-status, there is the subscriptionId given, but not in the corresponding component.
This will be fixed with this PR.

@maxl2287 maxl2287 changed the title fix: add missing "subscriptionId" in "RoamingStatus" fix: add missing "subscriptionId" in "RoamingStatus"-data for CloudEvent Feb 7, 2024
bigludo7
bigludo7 previously approved these changes Feb 8, 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.

Good Catch !

@sachinvodafone
Copy link
Collaborator

Hi @maxl2287 , It appears that this pull request aims to address two issues. First, correcting the device object where the country code was incorrectly placed under the device object. Second, ensuring that the country name is represented as an array, not a string, in the callback event . As per my understanding, It does not involve adding or amending the subscription field, as indicated in the request description, am i correct ?

@maxl2287
Copy link
Contributor Author

maxl2287 commented Feb 8, 2024

@sachinvodafone you're right!
That fixes with the country-code came in caused by the previous PR: #106

Thanks for the hint!

@maxl2287 maxl2287 marked this pull request as draft February 13, 2024 13:57
@maxl2287 maxl2287 marked this pull request as ready for review February 13, 2024 14:00
@maxl2287 maxl2287 requested a review from bigludo7 February 13, 2024 14:00
bigludo7
bigludo7 previously approved these changes Feb 13, 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

akoshunyadi
akoshunyadi previously approved these changes Feb 27, 2024
@fernandopradocabrillo
Copy link
Collaborator

Just one comment here. Shouldn't this be a fix? instead of going in the v0.6.0-wip, couldn't we include it in a v0.5.1?

@maxl2287 @bigludo7 @akoshunyadi

@fernandopradocabrillo
Copy link
Collaborator

Just one comment here. Shouldn't this be a fix? instead of going in the v0.6.0-wip, couldn't we include it in a v0.5.1?

@maxl2287 @bigludo7 @akoshunyadi

any thoughts? Once a team starts to implement the 0.5.0 version it's going to be obvious that there is a bug and I don't think that waiting to release a 0.6.0 is the best option. wdyt?

@maxl2287 @bigludo7 @akoshunyadi

@maxl2287
Copy link
Contributor Author

maxl2287 commented Mar 5, 2024

Just one comment here. Shouldn't this be a fix? instead of going in the v0.6.0-wip, couldn't we include it in a v0.5.1?
@maxl2287 @bigludo7 @akoshunyadi

any thoughts? Once a team starts to implement the 0.5.0 version it's going to be obvious that there is a bug and I don't think that waiting to release a 0.6.0 is the best option. wdyt?

@maxl2287 @bigludo7 @akoshunyadi

I aggree.

@bigludo7 @akoshunyadi what about 0.5.1-wip ?

@akoshunyadi
Copy link
Collaborator

The general idea in Camara is to make more -smaller- releases, so 0.6.0 should come quite soon, I would say. But I don't have any objection, if somebody needs a 0.5.1 even sooner.

@maxl2287 maxl2287 dismissed stale reviews from akoshunyadi and bigludo7 via 16a19d7 March 5, 2024 20:53
@maxl2287 maxl2287 requested review from akoshunyadi and bigludo7 March 5, 2024 20:54
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.

Good for me. Thanks

@akoshunyadi
Copy link
Collaborator

I'll create an issue for 0.5.1, where we can discuss and define the content of it.

@akoshunyadi akoshunyadi mentioned this pull request Mar 6, 2024
@akoshunyadi akoshunyadi mentioned this pull request Mar 6, 2024
@akoshunyadi akoshunyadi merged commit aec7a8c into camaraproject:main Mar 13, 2024
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.

5 participants