Skip to content

Replace Request with Axios#190

Closed
brianphillips wants to merge 2 commits intonodevault:masterfrom
brianphillips:replace-rp-with-axios
Closed

Replace Request with Axios#190
brianphillips wants to merge 2 commits intonodevault:masterfrom
brianphillips:replace-rp-with-axios

Conversation

@brianphillips
Copy link
Copy Markdown

This is a rebasing/fixup of #151.

One feature added after the original PR was submitted was the rpOptions configuration option which allows any option to be passed through to request-promise (see #125). This is arguably a bit of a leaky abstraction and would be difficult to implement with any degree of completeness (it would require a full mapping of request options to axios options which seems beyond the scope of this module). It seems like the original intent was just to allow an agent to be passed through to the underlying request library (which could be easily accomplished). We could also consider allowing various Axios-specific options to be passed through but regardless, for now, I've just skipped the existing test that exercised the rpOptions pending specific direction for how compatible we need to keep the API given the challenges I've mentioned.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 16, 2021

Codecov Report

Merging #190 (fc070bc) into master (18597dd) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #190   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          146       159   +13     
  Branches        37        42    +5     
=========================================
+ Hits           146       159   +13     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18597dd...fc070bc. Read the comment docs.

@brianphillips
Copy link
Copy Markdown
Author

@kr1sp1n Could you please let me know how to proceed, specifically with the rpOptions issue with removing request-promise support?

@aviadhahami
Copy link
Copy Markdown
Collaborator

@brianphillips see my comment here: #150 (comment)

Comment thread yarn.lock Outdated
@aviadhahami
Copy link
Copy Markdown
Collaborator

Closing in favor of #150

@brianphillips brianphillips deleted the replace-rp-with-axios branch November 10, 2022 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.

4 participants