Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
fernandopradocabrillo
left a comment
There was a problem hiding this comment.
LGTM! Thanks @bigludo7! I've corrected a minor typo, the rest is fine from my side
hdamker
left a comment
There was a problem hiding this comment.
The release PR is looking good, only point you might want to consider is to mention that this only a patch release of the previous version.
Regarding the content in main some minor issues - which you might want to consider for a next release:
AuthenticationId example and length suggests UUID but no format constraint, no indication of uniqueness guarantees:
AuthenticationId:
type: string
description: unique id of the verification attempt the code belongs to.
maxLength: 36
example: ea0840f3-3663-4149-bd10-c7c6b8912105
Typos in .feature files:
sendCode, line 107: max_lenght instead max_length
validateCode, line 166: calidate-code instead of validate-code
The use of 403 for business logic errors looks not compliant with HTTP and CAMARA (should be 422/409)
ONE_TIME_PASSWORD_SMS.MAX_OTP_CODES_EXCEEDED
ONE_TIME_PASSWORD_SMS.PHONE_NUMBER_NOT_ALLOWED # Validation issue - should be 422
ONE_TIME_PASSWORD_SMS.PHONE_NUMBER_BLOCKED # Business rule - could be 422
|
Thanks @hdamker |
|
Hello @hdamker - did you see any blocking point(s) for this one? |
hdamker
left a comment
There was a problem hiding this comment.
Thanks @bigludo7
Approved on behalf of Release Management 👏
Next steps for the team:
• [ ] PR merged (by API repository codeowner)
• [ ] Release created within GitHub (by API repository codeowner)
• [ ] Release Tracker updated (with creation date of the release and the release tag link)
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Publication of Fall'25 M4 public release of OTPValidation v1.1.1
Which issue(s) this PR fixes:
Fixes #114
Special notes for reviewers:
Changelog input
Additional documentation
This section can be blank.