Skip to content

Add a Schema option for JsonProvider#1523

Merged
dsyme merged 2 commits intofsprojects:mainfrom
EverybodyKurts:feature/json-schema-provider
Mar 11, 2025
Merged

Add a Schema option for JsonProvider#1523
dsyme merged 2 commits intofsprojects:mainfrom
EverybodyKurts:feature/json-schema-provider

Conversation

@EverybodyKurts
Copy link
Copy Markdown
Collaborator

This pull request adds a Schema option to JsonProvider that mirrors the Schema option for XmlProvider. It accepts either a JSON Schema string or file and like XmlProvider, it allows the developer to access JSON documents in a statically typed way.

Please note that I used claude.ai to help me write this code, tests, and documentation.

@EverybodyKurts EverybodyKurts force-pushed the feature/json-schema-provider branch from a4e82a7 to b406d46 Compare March 11, 2025 01:44
@EverybodyKurts
Copy link
Copy Markdown
Collaborator Author

Hi @dsyme and @cartermp, how should I request a review for this pull request?

@dsyme
Copy link
Copy Markdown
Contributor

dsyme commented Mar 11, 2025

@EverybodyKurts I'll do the review. @cartermp Would be great if you could take a look too

@dsyme
Copy link
Copy Markdown
Contributor

dsyme commented Mar 11, 2025

@EverybodyKurts It's a really great piece of work, also updating the engineering. Would you like to be co-maintainer on this repo going forward too? Thanks 🚀 😄 🙏


/// Specifies the formatting behaviour of JSON values
[<RequireQualifiedAccess>]
// [<RequireQualifiedAccess>]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think maybe this should not have been commented out?

@@ -0,0 +1,64 @@
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was wondering if this should also covre URIs and any other types in the JSON schema type

Copy link
Copy Markdown
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

@EverybodyKurts This is an incredible piece of work - really clean and I love that you've both added the testing and updated the engineering.

I've left two small comments, I think they can be addressed in follow-up PR

@dsyme dsyme merged commit a7de354 into fsprojects:main Mar 11, 2025
@EverybodyKurts
Copy link
Copy Markdown
Collaborator Author

@EverybodyKurts It's a really great piece of work, also updating the engineering. Would you like to be co-maintainer on this repo going forward too? Thanks 🚀 😄 🙏

@dsyme It would be an honor and a pleasure. I appreciate the work you and the developers on this project have done over the years.

This is a feature I've been wanting to add for awhile. I'm glad I was able to help contribute to it in a meaningful way.

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.

2 participants