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

Only enforce query object parsing when specified, always validate #186 #187

Merged
merged 9 commits into from Dec 18, 2019

Conversation

JacobLey
Copy link
Collaborator

This updated test+parsing highlights when forced JSON.parsing is not appropriate, and should rely on validator to determine success.

Opened as an attempt to solve this issue: #186

I tried my best to match existing code style, but wasn't sure if there was an official linter command/test. If so that has not been run but I am happy to tweak if necessary.

@JacobLey
Copy link
Collaborator Author

Ah looks like Travis does the linting

Linter wanted a comma, but that seemed wrong... As that implies function takes another argument.
This should appease the linter gods.
This reverts commit 7adaf2e.
@JacobLey
Copy link
Collaborator Author

I'm stumped on the linting... I can't figure out the run-local command so I don't pollute this PR with attempts.

In the last commit I was trying to replicate this test's usage of response.body: https://github.com/cdimascio/express-openapi-validator/blob/master/test/response.validation.spec.ts#L88

@JacobLey
Copy link
Collaborator Author

Apologies for the linter noise, I can re-open a PR/force squash those commits to clean it up if necessary

const schemaHasObject = schema => {
return (
schema.type === 'object' &&
schema.type !== 'array'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was copy+paste... but realistically is unnecessary (tautology)

Copy link
Owner

@cdimascio cdimascio 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. couple minor nits

test/resources/serialized.objects.yaml Show resolved Hide resolved
test/resources/serialized.objects.yaml Show resolved Hide resolved
@cdimascio
Copy link
Owner

cdimascio commented Dec 18, 2019

This updated test+parsing highlights when forced JSON.parsing is not appropriate, and should rely on validator to determine success.

Opened as an attempt to solve this issue: #186

I tried my best to match existing code style, but wasn't sure if there was an official linter command/test. If so that has not been run but I am happy to tweak if necessary.

in terms of linting. i use prettier to autoformat the code. .prettierrc.json is in the project root

@cdimascio cdimascio merged commit 75ca07b into cdimascio:master Dec 18, 2019
@cdimascio
Copy link
Owner

@all-contributors add @JacobLey for code and test

@allcontributors
Copy link
Contributor

@cdimascio

I've put up a pull request to add @JacobLey! 🎉

JacobLey added a commit to JacobLey/express-openapi-validator that referenced this pull request Dec 18, 2019
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.

None yet

2 participants