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

Public Release for Fall24 meta-release #139

Merged
merged 31 commits into from
Sep 6, 2024
Merged

Public Release for Fall24 meta-release #139

merged 31 commits into from
Sep 6, 2024

Conversation

bigludo7
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • subproject management

What this PR does / why we need it:

Related to issue #107

Which issue(s) this PR fixes:

Fixes #

Special notes for reviewers:

Missing link for Test Statement
PR for Test Definition no yet merged

Changelog input

 Public release number_verification v1.0.0

Additional documentation

This section can be blank.

docs

Copy link

github-actions bot commented Aug 26, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.03s
✅ OPENAPI spectral 1 0 1.65s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY secretlint yes no 0.71s
✅ YAML yamllint 1 0 0.3s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link
Contributor

@hdamker hdamker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one suggestion and one comment.

Please add @camaraproject/release-management_maintainers, as it is ready for review.

CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 13 to 14
| 7 | Basic API test cases & documentation | O | M | M | M | Y | [PR #124](https://github.com/camaraproject/NumberVerification/pull/124/files) |
| 8 | Enhanced API test cases & documentation | O | O | O | M | Y | [PR #124](https://github.com/camaraproject/NumberVerification/pull/124/files) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The status is either "tbd" with the PR, or you consider that PR 124 must be merged before the release PR, than you can already link here the coming .feature file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have just merged the Test Definition.
I've updated the rc1.2 branch accordingly.

@AxelNennker
Copy link
Collaborator

Please review my ICM review on 1.1
camaraproject/IdentityAndConsentManagement#189 (comment)

@rartych
Copy link
Contributor

rartych commented Aug 28, 2024

The link in the last line of README.md is wrong.
It should go to https://wiki.camaraproject.org/display/CAM/NumberVerification or https://wiki.camaraproject.org/x/jQOeAQ

Fixed link to meeting minutes (last ine)
Thanks @rartych
@hdamker
Copy link
Contributor

hdamker commented Aug 28, 2024

@bigludo7 please consider to fix #142 as part of this PR. Then all API YAMLs across the meta-release will have the same file name format.

Change number_verification to number-verification
Change name from number_verification to number-verification
Took into account file name change: number_verification.yaml to number-verification.yaml
@bigludo7
Copy link
Collaborator Author

@bigludo7 please consider to fix #142 as part of this PR. Then all API YAMLs across the meta-release will have the same file name format.

Done :)

@@ -5,7 +5,7 @@ Feature: Camara Number Verification API device phone number share

# Input to be provided by the implementation to the tests
# References to OAS spec schemas refer to schemas specified in
# https://raw.githubusercontent.com/camaraproject/NumberVerification/main/code/API_definitions/number_verification.yaml
# https://raw.githubusercontent.com/camaraproject/NumberVerification/main/code/API_definitions/number-verification.yaml
Copy link
Contributor

@hdamker hdamker Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is somewhere defined which version of the API definition the .feature file refers to? This link goes to main and therefore potentially into "wip".

Maybe a relative link would be better to stay within the tag/branch, but I don't think this is possibly for raw content. An alternative would be to link just to /code/API_definitions/number_verification.yaml

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've applied your suggestion.

@rartych
Copy link
Contributor

rartych commented Aug 30, 2024

In the path: section there is X-Correlator, the guideline was changed to use x-correlator (lower case)
https://github.com/camaraproject/Commonalities/pull/194/files

fixed X-Correlator to x-correlator.
@bigludo7
Copy link
Collaborator Author

In the path: section there is X-Correlator, the guideline was changed to use x-correlator (lower case) https://github.com/camaraproject/Commonalities/pull/194/files

Thanks to catch this one Rafal
Fixed.

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Update versions in test plans: Given the resource "/number-verification/v0 -> Given the resource "/number-verification/v1

bigludo7 and others added 3 commits September 3, 2024 14:27
changed "/device-phone-number/v0" to "/device-phone-number/v1"
changed /number-verification/v0 to /number-verification/v1 in the background section
@bigludo7
Copy link
Collaborator Author

bigludo7 commented Sep 3, 2024

  • Update versions in test plans: Given the resource "/number-verification/v0 -> Given the resource "/number-verification/v1

Thank @jlurien
fixed

@jlurien
Copy link
Collaborator

jlurien commented Sep 4, 2024

Let's wait to decision on camaraproject/ReleaseManagement#89 to merge this

Added sentences in doc as mentioned in #116
Add a line to indicate that we updated the documentation part of the yaml
@bigludo7
Copy link
Collaborator Author

bigludo7 commented Sep 5, 2024

Following discussion in today WG meeting & sync with @hdamker I've updated the yaml to add a clarification before the sequence diagram. Changelog has been also updated.
@hdamker @fernandopradocabrillo @jlurien I will need your approval (I understood @AxelNennker is off this week).
Thanks.

@tanjadegroot
Copy link

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

Taking care of the Test result statement accordingly to camaraproject/ReleaseManagement#89
@bigludo7
Copy link
Collaborator Author

bigludo7 commented Sep 5, 2024

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

Thanks @tanjadegroot
Applied
cc @hdamker

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release PR approved (on behalf of Release Management)

@bigludo7 PR ready to be merged and r1.2 can be created.Please update https://wiki.camaraproject.org/display/CAM/NumberVerification++API+Release+Tracking with date for M4 and link to r1.2 when it is created. Thanks

@jlurien jlurien self-requested a review September 6, 2024 09:21
Copy link
Collaborator

@fernandopradocabrillo fernandopradocabrillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bigludo7 bigludo7 merged commit 714ac37 into main Sep 6, 2024
2 checks passed
@hdamker hdamker deleted the bigludo7/r1.2 branch September 6, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants