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

[Go] Make dotprompt take in jsonschema.Schema instead of map[string]any for better ergonomics. #645

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

apascal07
Copy link
Collaborator

Description here...

Checklist (if applicable):

  • Tested (manually, unit tested, etc.)
  • Changelog updated
  • Docs updated

@jba
Copy link
Contributor

jba commented Jul 17, 2024

My main concern here is making github.com/invopop/jsonschema.Schema part of the API. I don't know if we do that anywhere else, but we shouldn't.

The problem is that that package is broken. It doesn't handle all JSONSchemas, and it probably never will. It's bad enough that we use it internally, but at least we can change that. If it's part of the API, replacing it will be a breaking change.

@apascal07
Copy link
Collaborator Author

My main concern here is making github.com/invopop/jsonschema.Schema part of the API. I don't know if we do that anywhere else, but we shouldn't.

The problem is that that package is broken. It doesn't handle all JSONSchemas, and it probably never will. It's bad enough that we use it internally, but at least we can change that. If it's part of the API, replacing it will be a breaking change.

Valid concern but we do have jsonschema.Schema part of the API already. Right above, we use it for InputSchema.

@jba
Copy link
Contributor

jba commented Jul 17, 2024

Oh yeah. Well that's too bad.

I think replacing that package should block going to v1.0. Unfortunately it won't be easy.

@apascal07 apascal07 merged commit 3318226 into main Jul 18, 2024
5 checks passed
@apascal07 apascal07 deleted the ap-go-boilerplate branch July 18, 2024 15:33
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