Skip to content

Conversation

szmarczak
Copy link
Collaborator

@szmarczak szmarczak commented Nov 5, 2019

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates.

@sindresorhus sindresorhus changed the title Deprecate legacy Url usage Deprecate legacy Url usage, drop query option Nov 7, 2019
@szmarczak szmarczak force-pushed the deprecate-legacy-url branch from c919fee to b803076 Compare November 7, 2019 18:58
@szmarczak szmarczak changed the title Deprecate legacy Url usage, drop query option Major refactoring Nov 11, 2019
@szmarczak

This comment has been minimized.

@szmarczak
Copy link
Collaborator Author

Okay, the only thing to do is to review documentation. Make necessary changes, then it's good to merge.

@szmarczak szmarczak marked this pull request as ready for review November 14, 2019 19:29
@szmarczak
Copy link
Collaborator Author

TODO: preNormalizeArguments should merge defaults - that can be done in a separate PR :)

Parsing method used to retrieve the body from the response. Can be `'default'`, `'text'`, `'json'` or `'buffer'`. The promise has `.json()` and `.buffer()` and `.text()` functions which set this option automatically.
Parsing method used to retrieve the body from the response.

- `'default'` - if `options.encoding` is `null`, the body will be a Buffer. Otherwise it will be a string unless it's overwritten in a `afterResponse` hook,
Copy link
Owner

Choose a reason for hiding this comment

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

Should we remove the null thing to get buffer as the user can just do responseType: 'buffer' instead? Which is also much clearer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll send a PR.

@sindresorhus
Copy link
Owner

Really nice work on this!! 🙌 I'm glad you got to this before the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment