Release PR r2.2 (second release candidate) #169
Conversation
Version & server.url Link into main branch replaced
hdamker
requested review from
bigludo7,
fernandopradocabrillo and
AxelNennker
as code owners
…-verification-API-Readiness-Checklist.md
There was a problem hiding this comment.
changes OK
some comments, but no mandatory changes
except maybe the missing mandatory section on device object in the yaml info.description section ? @hdamker please check.
other wise all is OK so I approve from Release Management
README.md
Outdated
There was a problem hiding this comment.
Structure of README is different from others - info on email list missing etc
e.g. move Meetings section lower and update with meetings info.
line 11: change "API family" to "APIs"; add API Sub project if applicable.
line 14: change "customer" to "API consumer"
line 20: remove from this section
There was a problem hiding this comment.
Created #170 for this comment.
There was a problem hiding this comment.
line 5: "Service Enabling Network Function API" --> "Service API"
line 26: where --> which
line 33: shorten to just "Note that the diagram shows just an example of a direct integration from developer's application", possibly add "and the API Provider's auth server"
line 54 is this line rellay needed ? (Ref to GSMA and mobile connect ? or keep only mobile connect ?
in the info.description section: is the mandatory sectio on Device object and corresponding error handling missing ? and also the related 422 errors ?
Move the externalDocs section under Info section
the yaml uses the data type "DevicePhoneNumber" - why not align to the CAMARA_common type "PhoneNumber" ? it has the same definition
There was a problem hiding this comment.
Created #171 for these suggestions, except for
in the info.description section: is the mandatory sectio on Device object and corresponding error handling missing ? and also the related 422 errors ?
which is actually not missing, see #169 (comment)
Thanks @tanjadegroot for the review. The functionality of NumberVerification is different from most other APIs, as it is actually the intend of the API to verify the phoneNumber, while the mandatory section on device object is describing the behavior to avoid that these APIs are as a side effect verify the phoneNumber. Therefore it is correct that the text is not added. The other suggestions for improvement I propose to leave to the team, the intend of the rc.2 is to be as similar as the rc.1 except of correcting the server url as requested in #168 |
There was a problem hiding this comment.
Thanks @hdamker @tanjadegroot, all, for the flexibility and support with this.
There was a problem hiding this comment.
LGTM.
Thanks @hdamker for the PR and @tanjadegroot for the review
|
@bigludo7 @AxelNennker would like to see one further code owner approval before I merge and create the release. |
|
@bigludo7 @AxelNennker taking silence in this case as approval :-) |
What type of PR is this?
What this PR does / why we need it:
/v1rc2/, in line with current guidelinesmainbranch with link intor2.2tagWhich issue(s) this PR fixes:
Fixes #168
Special notes for reviewers:
Please check if all necessary places are changed to be consistent