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

feat(reflect): Add method to modify the schema after processing #53

Merged
merged 4 commits into from
Nov 8, 2022

Conversation

webdestroya
Copy link
Contributor

This PR is very similar to #52 but moves the call to after the default schema builder has run.

This allows struct properties to be modified easily.

Copy link
Contributor

@samlown samlown 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! But I'm not convinced the interface method name is obvious enough. "Post" just doesn't say much without more context. How about JSONSchemaMod or JSONSchemaExtend?

reflect.go Outdated
// Function to be run after the schema has been generated.
// this will let you modify a schema afterwards
type postSchemaImpl interface {
JSONSchemaPost(*Schema)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not totally convinced on the name... "Post" just isn't obvious enough in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think JSONSchemaExtend is best. If you're good with that I can rename it

Copy link
Contributor

Choose a reason for hiding this comment

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

JSONSchemaExtension is perhaps more grammatically and semantically correct :-) (naming things 🤦 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I liked JSONSchemaExtend better, but your choice. Just let me know your final answer and I'll update the PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, lets go with JSONSchemaExtend!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, renamed

* main:
  Allow for = character in extra property values
  Add test for anyof
  Add support for parsing anyof_type
  Add tests for anyof support
  Add support for parsing anyof_required
Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks!

var customType = reflect.TypeOf((*customSchemaImpl)(nil)).Elem()
var extendType = reflect.TypeOf((*extendSchemaImpl)(nil)).Elem()
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same length as customType 😍

@samlown samlown merged commit 9f28aff into invopop:main Nov 8, 2022
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