Normalize options.method #547
Conversation
|
Can you add a test? |
|
And it's inconsistent to uppercase these while others are lowercased: Lines 18 to 23 in 3fd590a |
szmarczak
force-pushed
the
normalize-method
branch
from
10e819c
to
4faa58e
Compare
source/index.js
Outdated
| @@ -33,7 +33,6 @@ const defaults = { | |||
| }, | |||
| cache: false, | |||
| decompress: true, | |||
| useElectronNet: false, | |||
There was a problem hiding this comment.
Because it's not needed. See https://github.com/sindresorhus/got/blob/master/source/request-as-event-emitter.js#L43
There was a problem hiding this comment.
I think it's still useful to declare all options up-front, even if they are false by default.
There was a problem hiding this comment.
Ok, I'll leave that then.
| @@ -37,7 +37,8 @@ const create = defaults => { | |||
| return defaults.handler(normalizeArguments(url, options, defaults), next); | |||
| }; | |||
|
|
|||
| for (const method of defaults.methods) { | |||
| const methods = defaults.methods.map(method => method.toLowerCase()); | |||
There was a problem hiding this comment.
Why are you lowercasing them here, but uppercasing them in normalize-arguments?
There was a problem hiding this comment.
Why are you lowercasing them here,
Imagine calling got.POST instead of got.post :P
but uppercasing them in normalize-arguments?
To keep things consistent; because error.method is uppercased :)
There was a problem hiding this comment.
Nvm. I missed the context. It's late here...
There was a problem hiding this comment.
That's ok, answering questions is my job :P It's a bit late here too :)
No description provided.