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: parse payload on multipart/form-data request #8

Conversation

jo
Copy link
Contributor

@jo jo commented Jan 14, 2016

Closes #7

@dafortune
Copy link
Owner

Thanks so much! Great update! Could you add a quick test for this improvement? I have very little time this week. Otherwise I could probably do next week and merge

If you want, you can use https://github.com/daf-spr/hapi-qs/blob/master/tests/features/parse-payload.test.js as reference. Thanks!

@jo
Copy link
Contributor Author

jo commented Jan 28, 2016

Hi @daf-spr,

I changed the test you mentioned to test against both form data types.

})
};
if (isMultipart) {
requestOptions.formData = form
Copy link
Owner

Choose a reason for hiding this comment

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

Missing semicolon

@dafortune
Copy link
Owner

Great! Thanks, just a minor detail (a couple of missing semicolons) and it is good to go.

@jo jo force-pushed the parse-payload-on-multipart-form-data-requests branch from 6dc22a0 to 58ee9f3 Compare January 29, 2016 09:07
@jo
Copy link
Contributor Author

jo commented Jan 29, 2016

I added the two missing semicolons.

It would be great to include a linter which runs as part of the test. I recommend standard.

@dafortune
Copy link
Owner

Great! Thanks. Will create a separate issue to add a linter.

dafortune added a commit that referenced this pull request Feb 2, 2016
…orm-data-requests

feat: parse payload on multipart/form-data request
@dafortune dafortune merged commit dc511ec into dafortune:master Feb 2, 2016
@dafortune dafortune mentioned this pull request Feb 2, 2016
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