Prepare r1.3 Patch Release#38
Conversation
|
Just to avoid misunderstandings: I haven't done a full review, for that I have created camaraproject/ReleaseManagement#333 |
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
tanjadegroot
left a comment
There was a problem hiding this comment.
On fix on version in the test file.
on the format of the operationId names, I am checking if this can be part of a patch release or not. if not, then that should wait for the next release.
| @@ -1,4 +1,4 @@ | |||
| Feature: CAMARA Know Your Customer Fill-in API, vwip - Operation KYC_Fill-in | |||
| Feature: CAMARA Know Your Customer Fill-in API, v0.4 - Operation KYC_Fill-in | |||
There was a problem hiding this comment.
| Feature: CAMARA Know Your Customer Fill-in API, v0.4 - Operation KYC_Fill-in | |
| Feature: CAMARA Know Your Customer Fill-in API, v0.4.1 - Operation KYC_Fill-in |
There was a problem hiding this comment.
Per commonalities, the operationId should be in lowerCamelCase in this patch release, or else in the next initial release.
There was a problem hiding this comment.
I'll open an issue to fix it in teh next release. I think that it is a change that impacts too many things to go in a patch release.
There was a problem hiding this comment.
@tanjadegroot , @fernandopradocabrillo , does an upddate of yaml file with an operationId in lowerCamelCase may be included in a major version as the 2 string values are different (for case sensitive language) ?
I'm not sure it could have impact on existing source code ...
Gilles
There was a problem hiding this comment.
@GillesInnov35 changing the operationId is not a breaking change, it is not necessary to increase the major version when we change it since it is not part of the API contract. The endpoint and the ver remains the same, the operationId is an external identifier.
It might require an adaptation if any operator is using a code generator, but from the API perspective I think we should not consider it for a major change. In addition we are still in versions v0.X for Fill-in and in CAMARA so no need to go for a major, we can include it in v0.5.
There was a problem hiding this comment.
ok @fernandopradocabrillo , got it.
There was a problem hiding this comment.
Per Commonalities section 5.7.2, the operationId is defined in lowerCamelCase: For example: kycFillIn
see https://github.com/camaraproject/Commonalities/blob/main/documentation/CAMARA-API-Design-Guide.md#572-operations.
However I am not sure if you can change the operationId in a patch release. Checking with ohters.
Changing this also requires changing the same in the testfiles.
|
Also check if you can align to Commonalities latest release r3.4. (and update the API-readiness-checklist accordingly) |
|
One minor issue which I have just seen when testing some agent:
|
hdamker
left a comment
There was a problem hiding this comment.
To be consistent with the Readiness Checklist.
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
|
LGTM - leaving the final verdict from Release Management perspective to @tanjadegroot |
|
Hi @tanjadegroot, take a look whenever you can, thanks! |
tanjadegroot
left a comment
There was a problem hiding this comment.
All OK from ReleaseManagement
/LGTM
|
Hi @GillesInnov35, @ToshiWakayama-KDDI please approve so we can generate the patch. Thanks! |
ToshiWakayama-KDDI
left a comment
There was a problem hiding this comment.
Thank you, @fernandopradocabrillo , @tanjadegroot , @hdamker .
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Preparations for r1.3 patch release containing: