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

chore(request): Add an unfiltered option to getting full responses from npm #5667

Merged
merged 2 commits into from
Apr 15, 2018

Conversation

BYK
Copy link
Member

@BYK BYK commented Apr 13, 2018

Summary

Follow up to #5593. Looks like we use an npm-specific request header to reduce the bandwidth. That said, for some commands, we need the full responses, which was achieved by setting the Accept: application/json header manually. This patch introduces a more descriptive request option, unfiltered to standardize this usage.

See https://github.com/npm/npm-registry-client#requests for more info.

Test plan

Existing tests should pass.

**Summary**

Follow up to #5593. Looks like we need to send `Accept: application/json` header with most of the
registry requests so this patch adds it by default and removes the ad-hoc headers from info and
owner commands.

**Test plan**

Existing tests should pass.
@BYK BYK requested review from rally25rs and arcanis April 13, 2018 16:36
arcanis
arcanis previously approved these changes Apr 13, 2018
@arcanis
Copy link
Member

arcanis commented Apr 13, 2018

Seems like it breaks the yarn info command

@BYK BYK changed the title chore(request): Always send Accept: application/json header to registry chore(request): Add an unfiltered option to getting full responses from npm Apr 15, 2018
@BYK BYK dismissed arcanis’s stale review April 15, 2018 13:08

@arcanis - I've changed the PR quite a bit so I'd appreciate another review.

@BYK BYK merged commit 059ce44 into master Apr 15, 2018
@BYK BYK deleted the accept-json-ftw branch April 15, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants