Release PR for public release r1.2 (M4)

[jlurien]

What type of PR is this?

• documentation
• subproject management

What this PR does / why we need it:

Prepare all files for public release r1.2

Which issue(s) this PR fixes:

Fixes #250

Special notes for reviewers:

Pending tasks to be resolved before this one can be merged:

-API Readiness Checklists are already anticipating the target state with some tbd for pending tasks:

• Geofencing basic test plan: Geofencing feature file #181 ✅

• What to fill for ""Test result statement" for stable version of location-verification - Set to "N" with reference to open issue Clarify how to proceed with requirement "Test result statement" for stable APIs ReleaseManagement#89

• Changelog does no longer list r1.1, but this has to be clarified globally. Reverted

[github-actions]

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ ACTION	actionlint	2		0	0.03s
✅ OPENAPI	spectral	3		0	4.79s
✅ REPOSITORY	git_diff	yes		no	0.01s
✅ REPOSITORY	secretlint	yes		no	0.78s
✅ YAML	yamllint	3		0	0.77s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[hdamker]

• Changelog does no longer list r1.1, but this has to be clarified globally.

@jlurien @akoshunyadi
We should discuss this within Release Management. Personally I tend to add just to the top and don't edit the history of (pre-)releases. My points:

• The change lists are almost never identical, e.g. in this case r1.1 there wasn't the user story and the feature file for Geofencing. With enough time there would have been a second release candidate with the additional changes, so that someone who had worked with the first rc candidate hast also a changelog for the a) the rc.2 b) between rc.1 and public release.
• The public release is then the sum of all changes since the last public release, it could (or maybe even: should?) be written with another audience in mind then the release notes for release candidates. We haven't done that yet, but the release notes of a public release should explain the differences for API users, e.g. new features etc, while the release candidates could address more the information needs of implementations

About keeping the history you can argue that the release notes of pre-releases are still available within the CHANGELOG.md of the pre-release itself. But then the CHANGELOG file is getting less a log of the release history, but more a RELEASENOTE file finally.

Do you have examples of prominent projects and how they are handling there CHANGELOGs?
kubernetes as an example is keeping the complete history of pre-releases: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.30.md (just that they use a log of tooling and a new changelog per "major" release)

[jlurien]

Current indications in RM are:

• the update of the Changelog.md in the repository with new content on all APIs at the top for the most recent (pre-)release as follows:
  • for the first alpha or release-candidate API version, all changes since the release of the previous public API version
  • for subsequent alpha or release-candidate API versions, the delta with respect to the previous pre-release
  • for a public API version, the consolidated changes since the release of the previous public API version

We just need to explicitly clarify if intermediate pre-release versions should be kept or not. I don't see much value for this meta but maybe for future metas it may be useful.

Ideally the public release should be identical to the last rc.n in functionality but for this meta we are adding some last minute content to r1.2 that were not in r1.1. In this meta, it will not be easy to distinguish the (small) changes between r1.1 and r1.2 in any case.

[hdamker]

@jlurien the API_Release_Guidelines.md are even more explicit here:

add a new section at the top of the file for the release

There is nothing about overwriting previous pre-releases or changing the history within the CHANGELOG (it's the nature of a log that content is only added).

I propose not to question this guideline for the current Meta-release to avoid confusion across the APIs.

[jlurien]

Fine with that

[jlurien]

@hdamker,

I have updated the changelog, with r1.1 back in there. I think it would be an enhancement to reflect clearly per release since which version is the changelog consolidated, as kubernetes does for example:
https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.31.md#changelog-since-v1300

Now I have just added a sentence in bold in the Release Notes

[hdamker]

@jlurien I have already created the release review issue within Release Management to have the complete list there. Please declare "Ready for review" soon and add @camaraproject/release-management_maintainers as reviewer then.

[jlurien]

PR for r1.2 ready. Only pending issue: camaraproject/ReleaseManagement/issues/89

Thanks @bigludo7 @maxl2287 @hdamker

[bigludo7]

Thank a lot @jlurien to drive this work !!
Look good for me but I guess now the approval is in the hands of the release management team

[maxl2287]

LGTM :)

[jlurien]

@soadeyemo please approve also the PR here when you think it is ready to be merged, aligned with camaraproject/ReleaseManagement#90

[tanjadegroot]

Please see camaraproject/ReleaseManagement#89 on how to update the Test result statement in the API readiness checklist for this Fall24 meta-release.

[hdamker]

Please see camaraproject/ReleaseManagement#89 on how to update the Test result statement in the API readiness checklist for this Fall24 meta-release.

This applies (only) for the stable verify-location API (where the Test Statement is mandatory.

[bigludo7]

@hdamker I do not follow with enough attention QoD to answer you.
For /subscriptions in device-status we discussed in this issue camaraproject/DeviceStatus#191

As we provide subscription resource representation in the response, we wanted to make sure that the identifier is valued in this response (the API consumer needs to know for which device the subscription is requested). As such it was easier to keep requesting the identifier in the request.

If this topic needs to be reopen/rediscussed better to open an issue in Commonalities.

[soadeyemo]

All now OK. LGTM from release mgmt.
Update the tracker and please create the release.

[jlurien]

@hdamker @bigludo7 Regarding the last comments about device being required in geofencing, I think it make sense to open a dedicated discussion for this across all affected APIs, As Herbert comments, it makes also sense to include the same clarification that we added to QoD, explaining explicitly that any returned identifier must have been provided when creating the subscription, and with the same values, to avoid disclosing unnecessary information (e.g. IP/port are provided and a MSISDN is returned). Schemas are not affected but it is convenient to remark this requirement in the description.

As Device Status has already created r1.2, I prefer to defer any change to a later release and apply them across all similar APIs.

[tanjadegroot]

Hi, I noted the following(mostly minor) comments:

yaml files

1. in the location-verification.yaml, line 52 has: ".. the Telco Operator exposing the API" -> recommend to change to "... the API provider", as it can be Telco or Channel Partner).
2. Same point in the location-retrieval.yaml line 59
3. Same point in geofencing-subscriptions.yaml line 68

I think this is also an issue to be fixed in the template text - need to find where that is.

Feature files

1. in the location-verification.feature file, line 12 refers to the location-verification.yaml 0.2.0 --> is this on purpose ? if not I would drop the version there and make the file generic.
2. The same issue appears in the location-retrieval.feature file. In that file as well the version in line 1 should be the full version (0.3.0) IMHO.
3. Avoid using "customer" in descriptions, readme, etc: Hence, use "(API) consumer" iso "customer" in geofencing-subscriptions.yaml line 9, location-verification.yaml lines 5, 9, ... and location retrieval line 9;
4. similar avoid the word "subscriber" - change to "device" (e.g.in location-retrieval.yaml line 14)

User story:

1. In precondition: actor: should not refer to CSP (only), but more generically to "API provider" - this seems to be an issue in the template text in general.
2. In activities/Steps: Not clear if the ASP:User is the same as the End-User - use only one term.
3. Post-condition: I do not understand this sentence. not clear what is relation between ASP:User and End-User ?

Except for the 0.2.0 bug in the 2 feature files which needs to be fixed, these changes are improvement suggestions for the understanding of the API consumers. @jlurien up to you if you fix them now or in next release.

[tanjadegroot]

for geofencing, and the other subscriptions in DeviceStatus, I see that the device in the subscriptionDetail is still required at all.

That is currently consistent across all subscriptions APIs ... I recommend to postpone any change here into the next release cycle as this needs an analysis of the consequences. It will be a breaking change, as device is also required within all notifications (for whatever reason).

If it may help, per the API versioning guidelines, changing a mandatory parameter into an optional one is not a breaking change. In the other direction, optional to mandatory, it is a breaking change.

[hdamker]

I think this is also an issue to be fixed in the template text - need to find where that is.

@tanjadegroot that's indeed a template text, even a mandatory one, see https://github.com/camaraproject/IdentityAndConsentManagement/blob/r0.2.0/documentation/CAMARA-API-access-and-user-consent.md#mandatory-template-for-infodescription-in-camara-api-specs
It has to be changed there before it can be changed in the APIs. And there is already camaraproject/IdentityAndConsentManagement#190 to request here a change.

[hdamker]

1. in the location-verification.feature file, line 12 refers to the location-verification.yaml 0.2.0 --> is this on purpose ? if not I would drop the version there and make the file generic.

That has to be corrected ... but the feature file will not be generic but always belong to a concrete version of the API definition (as already indicated within line 1), at least for the newest changes added. Would be interesting to run later the v1.0.0 test definition against a (for example) API with version v1.5.0 and see if the backward compatibility is given.

[tanjadegroot]

I think this is also an issue to be fixed in the template text - need to find where that is.

@tanjadegroot that's indeed a template text, even a mandatory one, see https://github.com/camaraproject/IdentityAndConsentManagement/blob/r0.2.0/documentation/CAMARA-API-access-and-user-consent.md#mandatory-template-for-infodescription-in-camara-api-specs It has to be changed there before it can be changed in the APIs. And there is already camaraproject/IdentityAndConsentManagement#190 to request here a change.

@hdamker : OK, I referenced that issue in the new ICM issue #200 as there are a few other changes I propose in that new issue. Agree it needs to be fixed first.

[tanjadegroot]

1. in the location-verification.feature file, line 12 refers to the location-verification.yaml 0.2.0 --> is this on purpose ? if not I would drop the version there and make the file generic.

That has to be corrected ... but the feature file will not be generic but always belong to a concrete version of the API definition (as already indicated within line 1), at least for the newest changes added. Would be interesting to run later the v1.0.0 test definition against a (for example) API with version v1.5.0 and see if the backward compatibility is given.

@hdamker Agreed. However, I think one could only reference the API version once in the file. This would avoid such partial update mistakes.

[tanjadegroot]

Btw who is doing the final prep and merge of this PR ?

[hdamker]

Btw who is doing the final prep and merge of this PR ?

@camaraproject/device-location_codeowners ?

[jlurien]

Hi, I noted the following(mostly minor) comments:

yaml files

1. in the location-verification.yaml, line 52 has: ".. the Telco Operator exposing the API" -> recommend to change to "... the API provider", as it can be Telco or Channel Partner).
2. Same point in the location-retrieval.yaml line 59
3. Same point in geofencing-subscriptions.yaml line 68

Fair points, all changed.

I think this is also an issue to be fixed in the template text - need to find where that is.

I see Herbert responding on this

Feature files

1. in the location-verification.feature file, line 12 refers to the location-verification.yaml 0.2.0 --> is this on purpose ? if not I would drop the version there and make the file generic.
2. The same issue appears in the location-retrieval.feature file. In that file as well the version in line 1 should be the full version (0.3.0) IMHO.

They are errors, but better to remove version there, to reduce maintainance.

3. Avoid using "customer" in descriptions, readme, etc: Hence, use "(API) consumer" iso "customer" in geofencing-subscriptions.yaml line 9, location-verification.yaml lines 5, 9, ... and location retrieval line 9;
4. similar avoid the word "subscriber" - change to "device" (e.g.in location-retrieval.yaml line 14)

Proposal accepted and changed

User story:

1. In precondition: actor: should not refer to CSP (only), but more generically to "API provider" - this seems to be an issue in the template text in general.
2. In activities/Steps: Not clear if the ASP:User is the same as the End-User - use only one term.
3. Post-condition: I do not understand this sentence. not clear what is relation between ASP:User and End-User ?

I let @jgarciahospital to react on this feedback, but any change would need to be adopted in a future release

Except for the 0.2.0 bug in the 2 feature files which needs to be fixed, these changes are improvement suggestions for the understanding of the API consumers. @jlurien up to you if you fix them now or in next release.

Thanks for the feedback, please @tanjadegroot review if relevant comments are resolved

[jlurien]

I will merge the PR and create the release as soon as I get the final approval @hdamker @tanjadegroot

[jlurien]

@bigludo7 @maxl2287 I need as well another approval from you as the previous ones are dismissed after the last commit

[bigludo7]

LGTM

[tanjadegroot]

in line 1 of location-retrieval.feature: ... API, v0.3 --> v0.3.0

[tanjadegroot]

in line 1 location-verification.feature: ... API, v1.0 --> v1.0.0

[tanjadegroot]

and same in the geofencing feature file: always use full version nr when referencing.

[jlurien]

OK

[jlurien]

@tanjadegroot @bigludo7 sorry, review again as previous approvals are dismissed with any new commit

[bigludo7]

LGTM

[tanjadegroot]

All good now. LGTM from Release Management