Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
24048f5
This comment was marked as resolved.
This comment was marked as resolved.
|
Note on PR history: This PR was temporarily auto-closed when the head branch was renamed (GitHub treats it as the branch being deleted). The original branch has been restored and the PR is now re-opened. |
|
This PR has been converted from v0.1.0-alpha.2 to v0.1.0-rc.1, and we will use it as the M3 release. All version references have been updated consistently across the OpenAPI definition, docs, tests, changelog, and readiness checklist. |
tanjadegroot
left a comment
There was a problem hiding this comment.
Looks good ! Almost there.
Some small comments which you can include while waiting for the Spring26 Commonalities and ICM Release PR (ICM is available).
For Commonalities, please see here for API impacts: https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/518225924/Analysis+of+Commonalities+0.7.0-rc.1
a few of them concern your API so need to be addressed for alignment with the Spring26 meta-release.
This alignment also implies updating the readiness checklist to point to the release 4.1 of both Commonalities and ICM.
Suggest to
- put the Release PR in draft,
- do updates on main
- synch Release PR when done
- reopen Release PR
| | 1 | API definition | M | M | M | M | Y | [/code/API_definitions/click-to-dial.yaml](/code/API_definitions/click-to-dial.yaml) | | ||
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r2.3](https://github.com/camaraproject/Commonalities/releases/tag/r2.3) | | ||
| | 3 | Guidelines from ICM applied | O | M | M | M | Y | [r2.3](https://github.com/camaraproject/IdentityAndConsentManagement/releases/tag/r2.3) | | ||
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.3](https://github.com/camaraproject/Commonalities/releases/tag/r3.3) | |
There was a problem hiding this comment.
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.3](https://github.com/camaraproject/Commonalities/releases/tag/r3.3) | | |
| | 2 | Design guidelines from Commonalities applied | O | M | M | M | Y | [r3.3](https://github.com/camaraproject/Commonalities/releases/tag/r3.4) | |
There was a problem hiding this comment.
Note: There was a Commonalities maintenance release on documentation, but not impact on the API definition
|
|
||
| This section lists release notes for historical versions. | ||
| The current version on the main branch is **wip**. | ||
| The current version on the main branch is **v0.1.0-rc.1**. |
There was a problem hiding this comment.
Release notes should not point to main (which is always in version "wip".
There was a problem hiding this comment.
I would recommend not to duplicate release information in the API documentation as it is captured in the CHANGELOG.md and the README.md and will be handled automatically in the future.
Most API documentation should be in the API yaml file which should be self sufficient.
Additional info like the curl commands can be here to be part the release. Other information is up to you.
There was a problem hiding this comment.
I noticed the use of the "subject" property in the CloudEvent structure (line 558) which is not supported by CAMARA. as the same information is already in the "data" property of the event, it can simply be removed.
tanjadegroot
left a comment
There was a problem hiding this comment.
A fix is needed on all three feature files.
Also they do not cover all error cases today (see comment)
There was a problem hiding this comment.
- The resource URL is missing from the background section. E.g. add after line 10:
And the resource "/click-to-dial/v0.1rc1/calls"
- As x-correlator parameter is part of teh API, in the background section add!
And the header "x-correlator" complies with the schema at "#/components/schemas/XCorrelator"
Same comments on the other 2 feature files, but with resource adapted to .../{callId} and .../{callId}/recording respectively
There was a problem hiding this comment.
The different error codes are not covered by test scenarios which would be useful for API consumers. However, you can address this also in a later version. But would recommmend to set item 8 (Enhanced test cases) = in the API readiness checklist to "N" at this point.
|
The README.md was not part of the review, but there are some changes to be made:
|
|
Last but not least the API documentation (README, yamls, etc.) has information about an application that would use the API, rather then about the API, e.g.
Then there are error codes about information that does not seem to be covered/used by the API: INSUFFICIENT_BALANCE and RESTRICTED_DESTINATION. These probably should be removed for now and added in a later version of the API. |
|
Thanks @tanjadegroot for the thorough and detailed review — very helpful.
Planned PRs against main: PR-A: Test definitions
PR-B: OAS alignment
PR-C: README cleanup
PR-D: Commonalities/ICM alignment (pending final Spring26 baseline)
Updates in this release PR after syncing:
|
What type of PR is this?
What this PR does / why we need it:
This PR contains release-preparation only changes for v0.1.0-alpha.2:
This PR was created as a clean, reviewable replacement for the previous release PR #61, following Release Management guidance to avoid mixing CHANGELOG updates with substantial API/spec changes.
Special notes for reviewers: