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

Major refactoring #921

Merged
merged 35 commits into from Nov 16, 2019
Merged

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.

source/index.ts Outdated Show resolved Hide resolved
@sindresorhus sindresorhus changed the title Deprecate legacy Url usage Deprecate legacy Url usage, drop query option Nov 7, 2019
@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
source/utils/types.ts Outdated Show resolved Hide resolved
source/utils/types.ts Outdated Show resolved Hide resolved
source/normalize-arguments.ts Outdated Show resolved Hide resolved
source/progress.ts Show resolved Hide resolved
readme.md Show resolved Hide resolved
source/known-hook-events.ts Outdated Show resolved Hide resolved
test/hooks.ts Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
source/utils/types.ts Show resolved Hide resolved
source/utils/types.ts Outdated Show resolved Hide resolved
source/utils/types.ts Outdated Show resolved Hide resolved
test/arguments.ts Show resolved Hide resolved
test/arguments.ts Show resolved Hide resolved
source/utils/types.ts Outdated Show resolved Hide resolved
@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