Adding node-fetch target #154
Conversation
|
thanks for the PR @hirenoble haven't had time to check it out but this is a welcome target! |
|
@darrenjennings Been quite sometime since this PR was created. Anything I can help with to get this merged? Thanks 🙏 |
|
Anyone I can bribe with a tree donation to get this moving again? |
reynolek
left a comment
reynolek
left a comment
There was a problem hiding this comment.
Other than the accidental addition of .vscode/settings.json, looks good.
| @@ -0,0 +1 @@ | |||
| {} No newline at end of file | |||
|
@darrenjennings @develohpanda it seems that the PR is almost done it just need to be rebased and we need to remove |
|
I'll have a look at this PR and merge it next week, then remove the settings file. 👍 |
|
Didn't notice the conflicts initially - I probably won't get to it anytime soon. @jgiovaresco, if you are still keen to work on this, as there hasn't been any activity from the PR creator I'd say to grab the commit from his fork (https://github.com/hirenoble/httpsnippet), fix up whatever needs to be done, and create a standalone PR from your fork. We can attribute the feature to both once it gets merged in. |
@hirenoble is the original author (Kong#154). I've just resolve conflicts and fix tests that were failing.
|
Closing in favor of #180. Thanks for laying the groundwork! |
@hirenoble is the original author (Kong#154). I've just resolve conflicts and fix tests that were failing. @develohpanda noticed that JSON body should be stringified (https://github.com/node-fetch/node-fetch#post-with-json) And looking at fetch options (https://github.com/node-fetch/node-fetch#fetch-options) I noticed that `json` attribute does not exist, so I've removed it.
@hirenoble is the original author (#154). I've just resolve conflicts and fix tests that were failing. @develohpanda noticed that JSON body should be stringified (https://github.com/node-fetch/node-fetch#post-with-json) And looking at fetch options (https://github.com/node-fetch/node-fetch#fetch-options) I noticed that `json` attribute does not exist, so I've removed it. Co-authored-by: Hiren Shah <hirenanandshah@outlook.com>
@develohpanda I don't see myself appearing in the contributors list, anyways I'm glad that this has been complete. 👍 |
Will be in changelogs once we push out a new release that includes this feature, we don't have any control over what github lists as contributors though, which AFAIK is automatically produced from the authors of merged PRs 🤗 |
|
@develohpanda I opened this PR in November 2019, I waited for almost a year to get this pushed, we could have at least waited for a week for my response before rushing the things 😅 |
|
Apologies 🙏 Given there was no activity since the review in August, I had assumed you were MIA, and assumed you would have received notifications of the activity over the last few weeks as you were the PR author. In retrospect, you were not tagged in any conversations so may have missed it. No harm intended, there was an offer for help and I took that offer with the information I had in hand. I believe if I manually change the author for the commit and force-push, it should attribute correctly, so I'll give that a go. |
@hirenoble is the original author (#154). I've just resolve conflicts and fix tests that were failing. @develohpanda noticed that JSON body should be stringified (https://github.com/node-fetch/node-fetch#post-with-json) And looking at fetch options (https://github.com/node-fetch/node-fetch#fetch-options) I noticed that `json` attribute does not exist, so I've removed it. Co-authored-by: Julien Giovaresco <dev@giovaresco.fr>
|
@develohpanda No worries at all, it was my first ever open source contribution so was pretty excited about that. If we can ensure this doesn't happen with anyone else, at least @mention would be super helpful. I get a ton of github notifications so may have missed, I need to get better at that too. 🥂 |
|
Duly noted 🌟 Please don't be discouraged to contribute to OSS because of this! I am exploring ways to ensure the correct author is attributed in cases like these where a PR was simply finalized by somebody else. If you'd like to contribute some more, pop over to https://github.com/Kong/insomnia as we have some issues tagged with Also, I think the GitHub UI |
7 participants
Hi Team,
As npm
requestpackage is no longer supported, it is essential to have code samples generated usingnode-fetch. I have attempted to write a target usingnode-fetch.