Generic Body Parser #561
Generic Body Parser #561
Conversation
There was a problem hiding this comment.
Awesome work getting this back into shape. I am for sure a 👍 on this, but had nits and wanted to agree with your comments about proposed changes. Before a final review time IMO we should land those changes as well but lets maybe give it a bit for other folks like Linus and Chris to have time to review before wasting time if they disagree on those changes.
|
@wesleytodd @ctcpip @jonchurch I’ve addressed the feedback from earlier comments. Please let me know if there’s anything else that needs improvement, but I believe it’s ready for review now. Thanks! |
|
Hi @wesleytodd, @LinusU, @jonchurch, I hope you're doing well! I wanted to kindly follow up and ask if there's any chance we could move forward with this PR. Let me know if there's anything needed on my end to help move things along. |
package.json
Outdated
| @@ -46,7 +46,6 @@ | |||
| "lint": "eslint .", | |||
| "test": "mocha --require test/support/env --reporter spec --check-leaks --bail test/", | |||
| "test-ci": "nyc --reporter=lcov --reporter=text npm test", | |||
| "test-cov": "nyc --reporter=html --reporter=text npm test", | |||
| "test-debug": "npm test -- --timeout 0" | |||
There was a problem hiding this comment.
I thought it was only added for debugging purposes. I can add it back if you want
…arsers should import genParser'
|
@ctcpip I rebased it again and removed the two unnecessary commits. Let me know if you want me to readd the |
|
|
||
| function createBodyParser (parse, options, defaultOptions) { | ||
| // Squash the options and the overrides down into one object | ||
| var opts = { ...defaultOptions || {}, ...options } |
There was a problem hiding this comment.
| var opts = { ...defaultOptions || {}, ...options } | |
| var opts = { ...defaultOptions, ...options } |
| var bytes = require('bytes') | ||
| var contentType = require('content-type') | ||
| var createError = require('http-errors') | ||
| var debug = require('debug')('body-parser:generic') |
There was a problem hiding this comment.
I suggest passing the debug function as a parameter to createBodyParser(). This will allows the created middlewares to use their namespaces, such as body-parser:json, for debug messages.
@ctcpip Could you please specify which commits you'd like me to remove? |
|
@Phillip9587 Please see my comment in the other PR |
|
clean squash and rebase here: #574 |
Rebase of #544 as alternative to #550 and #551
cc @wesleytodd @ctcpip