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

refactor: use default for samples over generic type #582

Merged

Conversation

julshotal
Copy link
Contributor

@julshotal julshotal commented Jan 20, 2022

🧰 Changes

When there's no example present, right now we just autofill the example sample with the type. This will use the default if it exists before falling back to a generic type-based sample

I used how we were taking the default for booleans over a generic value and turned it into a reusable function to either use the default if it 1) exists and 2) is the right type, otherwise it falls back onto a provided generic value

The type check probably isn't super necessary, but on the off chance someone tries to use a wacky default hopefully it'll prevent something from breaking

🧬 QA & Testing

see tests

@julshotal julshotal marked this pull request as ready for review January 22, 2022 00:52
@julshotal julshotal requested review from erunion and Dashron January 22, 2022 00:53
@erunion erunion added enhancement New feature or request refactor Issues about tackling technical debt labels Jan 22, 2022
Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

lgtm! merge whenever and lmk so I can tag a release

@julshotal julshotal merged commit c1b1c66 into main Jan 24, 2022
@julshotal julshotal deleted the refactor/use-default-in-responses-if-no-example-rm-3096 branch January 24, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor Issues about tackling technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants