Skip to content

Conversation

@dragidavid
Copy link
Collaborator

@dragidavid dragidavid commented Apr 17, 2025

This PR adds the missing file validation. This is not really related to json schema in general but more just something that we support with jsf.

The goal was to mimic what we do in v0 and to make the failing tests pass.

@dragidavid dragidavid self-assigned this Apr 17, 2025
@dragidavid dragidavid marked this pull request as draft April 17, 2025 17:11
@dragidavid
Copy link
Collaborator Author

Leaving this as draft for now since even though file validation works across the board, it causes some regression with the test suite we run against.

Digging..

@dragidavid dragidavid marked this pull request as ready for review April 22, 2025 09:42
Copy link
Collaborator

@antoniocapelo antoniocapelo left a comment

Choose a reason for hiding this comment

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

Nice job here!
Most of the code looks good and from the testing it's also working nicely. I've left:

  • some minor comments, not blockers
  • a suggestion for refactoring and simplifying the type validation logic (can be done separately)
  • a blocker comment about keeping the consistency on how we call the individual validator functions

Copy link
Collaborator

@antoniocapelo antoniocapelo left a comment

Choose a reason for hiding this comment

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

Looks good!

@dragidavid dragidavid merged commit 2feb459 into main Apr 23, 2025
2 checks passed
@dragidavid dragidavid deleted the rmt-1591-implement-file-validation branch April 23, 2025 09:47
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