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 #81

Merged
merged 23 commits into from
Sep 9, 2024
Merged

Conversation

bigludo7
Copy link
Collaborator

@bigludo7 bigludo7 commented Aug 23, 2024

What type of PR is this?

Add one of the following kinds:

  • subproject management

What this PR does / why we need it:

Related to #58

Which issue(s) this PR fixes:

Fixes #

Special notes for reviewers:

In the changelog I've added the elements from the v0.5.0 that were missing.
Missing link for Test Statement

Changelog input

Public release one-time-password-sms v1.0.0

Additional documentation

This section can be blank.

docs

Copy link

github-actions bot commented Aug 23, 2024

🦙 MegaLinter status: ✅ SUCCESS

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

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

README.md Outdated Show resolved Hide resolved
Co-authored-by: Herbert Damker <[email protected]>
@hdamker
Copy link
Contributor

hdamker commented Aug 23, 2024

Line 1 of the feature file is Feature: CAMARA OTPvalidationAPI, v:wip - should be updated

Update version to v1.0.0 in line 1
@bigludo7
Copy link
Collaborator Author

Line 1 of the feature file is Feature: CAMARA OTPvalidationAPI, v:wip - should be updated

Fixed ! Thanks

@hdamker
Copy link
Contributor

hdamker commented Aug 23, 2024

Please replace the text underneath the Readiness checklist table with the latest version from https://github.com/camaraproject/ReleaseManagement/blob/main/documentation/API-Readiness-Checklist.md

As also mentioned in camaraproject/Commonalities#266 (comment)

@hdamker
Copy link
Contributor

hdamker commented Aug 23, 2024

So far what I've seen, rest LGTM. Please add @camaraproject/release-management_maintainers as reviewer to trigger the official review by Release Management and remove me as reviewer.

@bigludo7
Copy link
Collaborator Author

So far what I've seen, rest LGTM. Please add @camaraproject/release-management_maintainers as reviewer to trigger the official review by Release Management and remove me as reviewer.

Thanks a lot for Herbert for the very helpful review!

@bigludo7 bigludo7 requested a review from a team August 23, 2024 10:16
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.

Thanks @bigludo7 and @hdamker !

@hdamker
Copy link
Contributor

hdamker commented Aug 23, 2024

@bigludo7 @fernandopradocabrillo Could you please check if the comment in camaraproject/IdentityAndConsentManagement#189 (comment) is relevant for the API spec?

@bigludo7
Copy link
Collaborator Author

@bigludo7 @fernandopradocabrillo Could you please check if the comment in camaraproject/IdentityAndConsentManagement#189 (comment) is relevant for the API spec?

For me, and understood that @jpengar has same conclusion, the 403 INVALID_TOKEN_CONTEXT error is not relevant for the OTP validation API as if the phone number is in the token the API is useless. Indeed this API UC is to be use when we cannot detect the phone number by ourselves. But perhaps i miss something.

@hdamker
Copy link
Contributor

hdamker commented Aug 23, 2024

For me, and understood that @jpengar has same conclusion, the 403 INVALID_TOKEN_CONTEXT error is not relevant for the OTP validation API as if the phone number is in the token the API is useless. Indeed this API UC is to be use when we cannot detect the phone number by ourselves. But perhaps i miss something.

All good for me, I just wanted to be sure that the conclusion of @jpenpar is shared here.

@hdamker
Copy link
Contributor

hdamker commented Sep 4, 2024

.feature file - change of 36 lines needed:
/one-time-password-sms/v0/send-code -> /one-time-password-sms/v1/send-code
/one-time-password-sms/v0/validate-code -> /one-time-password-sms/v1/validate-code

Change all path from /v0/ to /v1/
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.

Checklist needs small updates & and a TSC decision about the missing prerequisite (which then can be added as comment).

@bigludo7
Copy link
Collaborator Author

bigludo7 commented Sep 5, 2024

@hdamker Managed the test statement in the checklist.

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

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 on behalf of @camaraproject/release-management_maintainers. Let's create the release.

@fernandopradocabrillo fernandopradocabrillo merged commit 83f96cc into main Sep 9, 2024
2 checks passed
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.

3 participants