Guidance for release PR review

Owned by Tanja de Groot
Last updated: Sept 11, 2024
5 min read

The following sections provide guidance on how to review a release PR / issue.

Please add questions as you come across them in the respective sections and add comments/text for answering them.

Review issues and process

  • Who can approve the M3 release PR ? Code Owners after RM approval is noted in the release PR or its issue. 

  • When to create a RM review issue ? when the release-management_maintainers list is included as a reviewer in a release PR. 

  • When do I tick a review issue task ?

    • Review actions: tick after checking the related file(s) and putting the review comment(s) in the PR/PR issue about the(se) files. Also put just OK for the file if no comments.

    • Also put a comment in the review issue with the major review comments and the overall state of the API.

    • Release actions: tick when task is done.

  • When do we close the review issue ? When all tasks in the review issue are ticked as done. Close as completed.

  • Question: should we add a milestone label so we can filter on it later ? 

  • How to know if an APIs is covered by RM ?  Use a comment in the comment fields of the API release tracker, visible on the Meta-release page.

Review actions

API definition files (YAML) (check version in info & servers objects)

  • Check Info object

    • version: aligned with the released API version and formatted correctly (per Commonalities)

    • description:

      • contains sufficient in-line API documentation

      • does not use Telco Jargon / abbreviations (non blocking for release if in documentation)

      • contains the section on auth from ICM 0.2.0 CAMARA-API-access-and-user-consent.md (with no link to ISM release in it - apply for next meta-release to API specs @Herbert Damker to check with ICM team.

      • avoids use of "customer", unless explicitly explained what is meant. Instead use as applicable API consumer, (application) End-user, or whatever else is applicable.

      • avoids use of "Telco/operator/CSP", as also Aggregators, etc. may expose the API. Use e.g. API Provider or API platform as applicable. 

      • avoids use of "subscriber". Use Device or End-user (defined as the application user) as applicable

      • avoids using customer: use API consumer or (application) End-user, as applicable

    • presence of the x-camara-commonalities release referenced, e.g. 0.4.0-rc.1 or 0.4.0.

  • Check servers object has the correct version format e.g. v0.xrc1, v1rc1, v0.x or v1.

  • Check security schemes: check presence and format (OIDC) and scope name format.

  • Is there a way to know if an API requires consent request ? No, this is local regulation. It is covered when using the security scheme and the scope rules.

  • In externalDocs: this field is optional, but recommend to point to the CAMARA, repo.
      description: Project documentation at CAMARA
      url: https://github.com/camaraproject/<repo>

    • @Tanja de Groot to create a Commonalities backlog issue

  • Shall error cases be explained in in-line documentation ? (info.description) It seems yes as per Commonalities/documentation/API-DocumentationTemplate.md - Deprecate the doc and doc shall be inline in OAS spec. @Rafal to updater in Commonalities

test file(s) availability

  • should the test file contain the version number ? e.g. Feature: CAMARA Call Fowarwing Signal  API, v0.1.0 - Operation call-forwardings - is it required to include the version (as people forget to update it). As the tests are in the same release package, can the version number be avoided in the test features ?

  • all error codes shall be covered by final test scenarios. what do we consider the main error cases for rc ? 

  • can we automate checking that all API spec errors/cases are covered by the tests ?

changelog updated

  • Comment: For capturing changes, its is recommended to use the GitHub release feature to get the list of all the changes which can then be selected as appropriate and put in the right categorie (added, changed etc) - ask a GitHub expert how to do this by creating a draft release (guidelines are WIP).

readme updated (enforce correct release number / API version naming)

  • enforce the correct release number / API version naming

  • avoid the use of the word "customer", unless explicitly explained what is meant. Alternative can be developer, (application) end-user, API consumer, etc. (as applicable).

  • avoid reference to CSPs in case also Aggregators, etc. may expose the API. Use e.g. API platform or exposure platform. 

API readiness checklist(s)

  • API definition files

  • commonalities

    • availability of linting report in release issue (tbc)

  • ICM

  • versioning (checked in yaml)

  • documentation (beyond in-line)

    • check for Telco jargon / abbeviations

  • user stories

  • basic test files (for M3)

    • is it necessary for a Feature to reference the API version ? as it is anyway in the same release package ? 

    • cover 200 response and all error responses (tbc)

  • enhanced test files (for M4)

    • all all error responses shall be covered by M4 

    • add guideline to use error codes in scenario names ?

  • release numbering

  • changelog

  • certification - for stable public APIs

Re-usable review comment texts

  • if you see a release PR that is nearly ready, add a comment as follows: "Please add @release-management_maintainers to this PR once the PR is ready"

  • for final approval use a comment: "LGTM from ReleaseManagement."



ICM review checks:

(from: https://github.com/camaraproject/IdentityAndConsentManagement/issues/189#issuecomment-2315026741)

Ref fro examples: https://github.com/camaraproject/SimpleEdgeDiscovery/blob/r1.2/code/API_definitions/simple-edge-discovery.yaml

Check the ICM-defined info.description template (Authorization and Authentication section). Reference
Example: Checked https://github.com/camaraproject/SimpleEdgeDiscovery/blob/r1.2/code/API_definitions/simple-edge-discovery.yaml#L203
Check the use of the security property according to ICM definitions. Reference
Example: Checked the one endpoint
https://github.com/camaraproject/SimpleEdgeDiscovery/blob/r1.2/code/API_definitions/simple-edge-discovery.yaml#L253
Error codes are defined by Commonalities e.g. INVALID_TOKEN_CONTEXT.
Example: https://github.com/camaraproject/SimpleEdgeDiscovery/blob/r1.2/code/API_definitions/simple-edge-discovery.yaml#L551
However, the ICM could check the definition of a 403 INVALID_TOKEN_CONTEXT if it applies to a specific API (e.g. APIs using device object or phoneNumber in the API request). Reflects an inconsistency between information in some field of the API request and the access token.
⚠️ OK with comments. The API specification has its own section Identifying the Device. The API specification does not include the recommended section called "Identifying a device from the access token" in info.description that provides a detailed description of the expected handling of the device object in the API request as it relates to the access token. It is specified in Appendix A: info.description template for device identification from access token and it is required for APIs that use the device object in the API requests.
@Kevsy @crissancas @javierlozallu please check whether the recommended section is applicable
Verify that there is no unexpected leakage of users' personal information, such as API responses containing identifiers or information beyond the API functionality. 

Example: OK SimpleEdgeCloud can be used to verify a phone number like NumberVerification does. Please see API misuse Commonalities#259. If Phone-Number is part of the SimpleEdgeCloud request then response tells the API consumer the same as a request to NumberVerification does.

For APIs including a device object, check that Appendix A of the API-Design-Guidelines.md is respected: info.description template for device identification from access token



ICM Review Result: Example: OK



Release actions

  • Tick task when checked and done.

  • Check if further review by TSC / Commonalities / ICM is needed (e.g. for targeted stable APIs), and leave issue open until those reviews are marked as done and OK in the review issue

  • When all tasks and complementary reviews are completed, close the review issue with a comment on the overall status of the API.