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

blockchain_identification_and_alignment_topics #30

Merged
merged 7 commits into from
Nov 30, 2023

Conversation

PedroDiez
Copy link
Collaborator

@PedroDiez PedroDiez commented Oct 31, 2023

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature
  • documentation

What this PR does / why we need it:

This PR is intended to cover:

  • Issue#19 [PROPOSAL DONE: Description and Exception. Parameter name pending]
  • Commonalities Alignment PR#57: securitySchema indication, scopes naming format
  • Clarification/rewording around term "phone number" -> "mobile phone number"
  • Clarification in "Tag" description
  • Adding "" to some example values
  • Regarding blockchainNetworkId defined two exceptions:

400 BLOCKCHAIN_PUBLIC_ADDRESS.INVALID_BLOCKCHAIN_NETWORK_IDENTIFIER. To control format (i.e. indicating a blockchainNetworkId not covered yet or not respecting the format agreed)
403 BLOCKCHAIN_PUBLIC_ADDRESS.BLOCKCHAIN_NETWORK_IDENTIFIER_NOT_ALLOWED. To control business case when a Telco Operator does not manage a valid blockchainNetworkId within their business logic.

Which issue(s) this PR fixes:

Fixes #19
Alignment with Commonalities#57

Special notes for reviewers:

Blockchain Identification parameter name set to blockchainNetworkId

@PedroDiez PedroDiez requested a review from a user October 31, 2023 20:52
@PedroDiez PedroDiez self-assigned this Oct 31, 2023
@PedroDiez PedroDiez added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 31, 2023
grgpapadopoulos
grgpapadopoulos previously approved these changes Nov 2, 2023
Co-authored-by: Sebastian Köller <sebastian.koeller@vodafone.com>
@PedroDiez
Copy link
Collaborator Author

Thanks @grgpapadopoulos, @sebKoeller for the review so far.
I will be adding the exception pending

@rartych
Copy link
Collaborator

rartych commented Nov 3, 2023

I guess it would be more correct to use the expression "associated with" instead of "associated to" in the descriptions.
There is again discussion in Commonalities about including headers like x-correlator in CAMARA OpenAPI files, when the issue camaraproject/Commonalities#77 is clarified we should align the specification here.
The same with operation tags - camaraproject/Commonalities#80

@PedroDiez
Copy link
Collaborator Author

I guess it would be more correct to use the expression "associated with" instead of "associated to" in the descriptions. There is again discussion in Commonalities about including headers like x-correlator in CAMARA OpenAPI files, when the issue camaraproject/Commonalities#77 is clarified we should align the specification here. The same with operation tags - camaraproject/Commonalities#80

Thanks Rafal!

  • Regarding the expression "associated with" instead of "associated to". I will be correcting it
  • Regarding the Commonalities Issues: Have taken note of them to make adaptations required based on their outputs

@PedroDiez PedroDiez requested review from a user and grgpapadopoulos November 23, 2023 19:18
@PedroDiez
Copy link
Collaborator Author

Thanks @grgpapadopoulos, @sebKoeller

Will merge it during meeting

@PedroDiez PedroDiez merged commit 8eab817 into main Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Solving the chain identification issue
3 participants