Better handling of boolean arguments #24
Conversation
bengourley
force-pushed
the
boolean-args
branch
from
8dedca1
to
88a2c65
Compare
bengourley
force-pushed
the
boolean-args
branch
from
88a2c65
to
4d11f1a
Compare
| // true, so only add it to the payload if it is truthy | ||
| if (options.overwrite) { | ||
| formData[name] = String(value); | ||
| } |
There was a problem hiding this comment.
If I make a curl call with overwrite=false, would the API interpret that value as truthy? If so, we should update our docs to make that clearer.
| @@ -297,6 +297,14 @@ function prepareRequest(options) { | |||
| case 'tempDir': { | |||
| break; | |||
| } | |||
| case 'overwrite': { | |||
There was a problem hiding this comment.
Does this only apply to overwrite, and not any of the other boolean flags?
There was a problem hiding this comment.
Only overwrite as that's the only one that gets sent to the API. All of the other boolean args configure the behaviour of this module but don't get put in the payload.
There was a problem hiding this comment.
Looks good to me, tests are a nice addition to the project.
The only other thing that I think needs altering is the wording of our API docs, as IMO they imply that setting overwrite as "false" will not overwrite existing maps. https://docs.bugsnag.com/api/js-source-map-upload/
overwriteparameter is not passed through to the API when it isfalse(this is because the body is a URL encoded form sofalsegets encoded as"false"which is a truthy value!)The latter point addresses #23.