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

refactor: drop needle dependency #1903

Closed
wants to merge 3 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented May 10, 2021

This will be a rough 1:1 migration from needle to Node's http/s modules. It will need refactoring to make the API more reusable.

  • Split response and body parsing. Not all users will need the body or may want to process it differently using the response stream directory.
  • Tidy up the difference between request/index (clever callback/promise api), request/request (promise api) and request/http (replacement for needle).
  • Do we need redirects? Node'shttp/s doesn't follow them for us like Needle did. See if tests pass without it.

@github-actions
Copy link
Contributor

github-actions bot commented May 10, 2021

Fails
🚫

Commit "wip: use new request module" is not a valid commitizen message. See Contributing page with required commit message format.

🚫

Commit "wip: migrate remaining tests to request/http" is not a valid commitizen message. See Contributing page with required commit message format.

Messages
📖

This PR will not trigger a new version. It doesn't include any commit message with feat or fix.

Generated by 🚫 dangerJS against 169678b

@ghost ghost force-pushed the refactor/remove-needle branch from 1594c5e to 44c7fbd Compare May 10, 2021 15:39
@maxjeffos maxjeffos force-pushed the refactor/remove-needle branch from 12e630d to 3051b60 Compare May 11, 2021 18:11
Jahed Ahmed and others added 2 commits May 12, 2021 10:35
Co-authored-by: Jeff McLean <maxjeffos@users.noreply.github.com>
Co-authored-by: Jahed Ahmed <jahed-snyk@users.noreply.github.com>
@ghost ghost force-pushed the refactor/remove-needle branch from 73b2a93 to 7459de4 Compare May 12, 2021 10:37
This pull request was closed.
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.

1 participant