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

Scope for RC.2 for Meta Release Fall 24 #72

Closed
FabrizioMoggio opened this issue Jul 23, 2024 · 19 comments · Fixed by #91
Closed

Scope for RC.2 for Meta Release Fall 24 #72

FabrizioMoggio opened this issue Jul 23, 2024 · 19 comments · Fixed by #91
Labels
enhancement New feature or request Fall24

Comments

@FabrizioMoggio
Copy link
Collaborator

FabrizioMoggio commented Jul 23, 2024

Problem description
0.2.0-rc.1 must be updated according to the comments from the Meta-Release reviewers
The scope RC.2 is the full alignment of the CFS API with the CAMARA Guidelines

Possible evolution
update the API according to the received comments

Additional context

@FabrizioMoggio FabrizioMoggio added enhancement New feature or request Fall24 labels Jul 23, 2024
@tanjadegroot
Copy link
Contributor

tanjadegroot commented Jul 25, 2024

API definition yaml file review comments:

in the info object:

@tanjadegroot
Copy link
Contributor

tanjadegroot commented Jul 25, 2024

[Test_definitions] in both feature files:

the version in the "Feature" line needs to be updated.

the background section still has "vwip"

Could just the resources be referenced in the background section (without the base path /call-forwarding-signal/version) ?

@tanjadegroot
Copy link
Contributor

tanjadegroot commented Jul 26, 2024

Comments on CHANGELOG.md:

line 10: first (pre) release candidate of --> first pre-release of
Explanation: "you pre--release and API version. The API version in this pre-release is a release-candidate. alpha and rc API versions are in pre-releases. At M5, you will release a initial public API version.

To have some info in the ToC of the changelog about wat is in the pre-release, suggest to change the section title and ToC item to "r1.1 - rc". See the Changelog example here.

line 21: This is an pre release candidate version ... --> This pre-release concerns a release-candidate API version ...

As foreseen in #59, you'll need to add the section "r1.2 - rc" to the CHANGELOG, with the delta wrt r1.1

@tanjadegroot
Copy link
Contributor

tanjadegroot commented Jul 26, 2024

README.md comments: all OK

@tanjadegroot
Copy link
Contributor

API Readiness Checklist comments:

suggestion to rename the file to have the full API name (call-forwarding-signal-API-Readiness-Checklist) and minimize the use of the abbreviation in general.

@tanjadegroot
Copy link
Contributor

User Stories comments:

  • the use of the term "Customer" is Telco focused. I would suggest using End-user instead.
  • same for "Operator Platform (OP)" which you could generalize to "API Provider (AP)" instead.
  • minimizing the (Telco habit) of abbreviations is also good

You may consider these as content comments rather then release mgmt comments, so please use as you see fit.

@tanjadegroot
Copy link
Contributor

Personal comment:

Given the explanations of the API, it seems to me that the name of this API could be simplified for easier understanding to "call-forwarding-status" as there is not really any use for the term "signal" anywhere, and it is all about status.

secondary comment: the resource name "call-forwardings" (which sounds like an action) could be changed to "call-forwarders" (which sounds more like an object).

Please use as you see fit.

@tanjadegroot
Copy link
Contributor

Release Management review done - Let me have a last check with the RM team before approval.

@FabrizioMoggio
Copy link
Collaborator Author

FabrizioMoggio commented Jul 29, 2024

API definition yaml file review comments:

in the info object:

@tanjadegroot just to be sure, "[0.4.0](x-camara-commonalities: 0.4.0)" already is in lines 148 and 149. Are you suggesting to move it in line 144?

I have placed it in line 148 according to https://github.com/camaraproject/Commonalities/blob/r0.4.0-rc.1/documentation/API-design-guidelines.md#info-object

ok for removing the contact field (and value): #76

@FabrizioMoggio
Copy link
Collaborator Author

[Test_definitions] in both feature files:

the version in the "Feature" line needs to be updated.

the background section still has "vwip"

Could just the resources be referenced in the background section (without the base path /call-forwarding-signal/version) ?

issue: #77

@FabrizioMoggio
Copy link
Collaborator Author

Comments on CHANGELOG.md:

line 10: first (pre) release candidate of --> first pre-release of Explanation: "you pre--release and API version. The API version in this pre-release is a release-candidate. alpha and rc API versions are in pre-releases. At M5, you will release a initial public API version.

To have some info in the ToC of the changelog about wat is in the pre-release, suggest to change the section title and ToC item to "r1.1 - rc". See the Changelog example here.

line 21: This is an pre release candidate version ... --> This pre-release concerns a release-candidate API version ...

As foreseen in #59, you'll need to add the section "r1.2 - rc" to the CHANGELOG, with the delta wrt r1.1

issue: #78

@FabrizioMoggio
Copy link
Collaborator Author

API Readiness Checklist comments:

suggestion to rename the file to have the full API name (call-forwarding-signal-API-Readiness-Checklist) and minimize the use of the abbreviation in general.

Issue: #79

@FabrizioMoggio
Copy link
Collaborator Author

User Stories comments:

  • the use of the term "Customer" is Telco focused. I would suggest using End-user instead.
  • same for "Operator Platform (OP)" which you could generalize to "API Provider (AP)" instead.
  • minimizing the (Telco habit) of abbreviations is also good

You may consider these as content comments rather then release mgmt comments, so please use as you see fit.

Issue:_ #81

@FabrizioMoggio
Copy link
Collaborator Author

Personal comment:

Given the explanations of the API, it seems to me that the name of this API could be simplified for easier understanding to "call-forwarding-status" as there is not really any use for the term "signal" anywhere, and it is all about status.

secondary comment: the resource name "call-forwardings" (which sounds like an action) could be changed to "call-forwarders" (which sounds more like an object).

Please use as you see fit.

Discussion: #82

@FabrizioMoggio
Copy link
Collaborator Author

Implemente

API definition yaml file review comments:
in the info object:

@tanjadegroot just to be sure, "[0.4.0](x-camara-commonalities: 0.4.0)" already is in lines 148 and 149. Are you suggesting to move it in line 144?

I have placed it in line 148 according to https://github.com/camaraproject/Commonalities/blob/r0.4.0-rc.1/documentation/API-design-guidelines.md#info-object

ok for removing the contact field (and value): #76

@tanjadegroot, please have a look on the above comment of mine. In my understanding the reference to commonalities version is in the right place. Is it ok for you?

@FabrizioMoggio
Copy link
Collaborator Author

Release Management review done - Let me have a last check with the RM team before approval.

@tanjadegroot once we have clarified #72 (comment). Can we consider the release approved?

@hdamker
Copy link
Contributor

hdamker commented Aug 6, 2024

@FabrizioMoggio @tanjadegroot is on well-deserved vacation, so let me take over here.

As far as I can see there is almost everything done already, distributed over different PRs.

A short reminder about the procedure:

  • all content changes (on documentation, YAMLs, ...) should normally done within their own PRs. These PRs should set the version number of the touched file always to "wip". The merge commits of these PRs are not released.
  • there should be ONE, final "Release PR" which should
    • update the CHANGELOG.md with a new section on the top of the file
    • update the README so that the new release and API Version is mentioned
    • update the API Readiness Checklist with status achieved with this release
    • set the version, x-camara-commonality url field within the API YAML(s)
    • if needed: set the version within the test definition files
      The reason to do all these changes together within one PR is to avoid inconsistent content within the repository. Directly after the merge of the release PR the release tag will be created

And: The release PR should not be merged before it was reviewed and approved by one Release Management maintainer. For this reason you should add @camaraproject/release-management_maintainers as a reviewer to the PR.

I will create now a release PR for you and add the last point(s) I will discover in my review as proposals.

@FabrizioMoggio
Copy link
Collaborator Author

FabrizioMoggio commented Aug 6, 2024

@hdamker actually all the documents are already released (also the final changelog). All the documents already refer to the final version and release tag (r0.2.1-rc and r1.2-rc). Only in the YAML we still have wip. That was my understanding, that the final commit, after all the approval, should have been with just the modification of the release number from wip to "r.xyz". I (wrongly :-)) assumed just in the OAS file.

@hdamker hdamker linked a pull request Aug 6, 2024 that will close this issue
@hdamker
Copy link
Contributor

hdamker commented Aug 6, 2024

@FabrizioMoggio all good ... you just have currently the situation that the README.md points to a release which does not exist. And the documents are "released" when they are in a tagged release commit of the repository ... so not yet released, but well prepared for the release.

Let us just do the remaining steps for the release now as soon as possible:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Fall24
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants