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: expose FromSchema options #22

Merged
merged 5 commits into from
Dec 13, 2022
Merged

feat: expose FromSchema options #22

merged 5 commits into from
Dec 13, 2022

Conversation

kalvenschraut
Copy link
Contributor

Checklist

There are not a lot of tests/documentation for this package or any benchmarks so not sure if the second and third checkboxes are relevant. If deemed needed I can look at doing something

Otherwise my goal with this PR is to be able to pass in shared references that we define. They are supported by the provider as described in their documentation here. All I need was the ability to pass through my references, which accomplished by adding the generic.

@kormang
Copy link

kormang commented Nov 13, 2022

Just wanted to make the same thing. Please, merge this. This is game changer, if you want to use Date throughout the code, and serialize/deserialize it to/from string.

I've already tried your code and it works.

@kalvenschraut kalvenschraut mentioned this pull request Nov 30, 2022
2 tasks
@JeffersonBledsoe
Copy link

Just came to +1 this!

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

Can you add an unit test on how the new option should behave?
And assert the correct output when you use it.

@kalvenschraut kalvenschraut reopened this Dec 10, 2022
@kalvenschraut
Copy link
Contributor Author

Didn't realize I was working off the main branch for this, sync'ed up with this repo and added docs and tests for specifically #25 as that was what my use case was also.

@RafaelGSS RafaelGSS merged commit 357ff54 into fastify:main Dec 13, 2022
@RafaelGSS
Copy link
Member

Thanks!

@kalvenschraut
Copy link
Contributor Author

kalvenschraut commented Dec 16, 2022

Any plans on doing a release soon? Would definitely prefer to track the main package again instead of my fork.

Edit

Now release, thanks!

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.

5 participants