-
Notifications
You must be signed in to change notification settings - Fork 34
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
Move parameters schema keywords location #108
Conversation
return { | ||
// @ts-ignore | ||
...rest, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to keep this spread of rest at the root of the return and not in schema
?
i'll be pulling and testing out tonight but wanted to give a review before that and ask ahead :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marclave
Had a thought of it and I think it makes more sense to be under schema
.
Most JSON schema keywords goes under schema
except for description
, required
, examples
. So there's lesser keywords to define
Also when we choose to define some keywords and send others to spread, it makes more sense to me to define generic keywords explicitly and send type-specific keywords like minLength
, which belongs to schema
, to the rest
@@ -27,17 +27,30 @@ it('returns a valid Swagger/OpenAPI json config for many routes', async () => { | |||
) | |||
.post( | |||
'/json/:id', | |||
({ body, params: { id }, query: { name } }) => ({ | |||
({ body, params: { id }, query: { name, email, birthday } }) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome! more tests :)
description
andexamples
as parameter keywords, while move others to schema keywords. Based on: