-
Notifications
You must be signed in to change notification settings - Fork 7
feat: enhance validation error messages for ADR-45 and ADR-173 compliance #329
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
Conversation
…larity and compliance with ADR-173
Test this pull request
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! 👍 I've left a couple of comments to review.
Co-authored-by: Lautaro Petaccio <[email protected]> Signed-off-by: Andrés Morelos <[email protected]>
Co-authored-by: Lautaro Petaccio <[email protected]> Signed-off-by: Andrés Morelos <[email protected]>
…ve clarity and consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (leaving a NIT comment)
src/validations/ADR45.ts
Outdated
| if (entityIsNotVersion3(entity) && entityWasDeployedAfterADR45(entity)) | ||
| return validationFailed('Only entities v3 are allowed after ADR-45') | ||
| return validationFailed( | ||
| 'Upload failed: only entities v3 are allowed after the ADR-45. Check http://adr.decentraland.org/adr/ADR-45 for more information' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'Upload failed: only entities v3 are allowed after the ADR-45. Check http://adr.decentraland.org/adr/ADR-45 for more information' | |
| 'Only entities v3 are allowed after the ADR-45. Check http://adr.decentraland.org/adr/ADR-45 for more information' |
fix #20