Skip to content
This repository has been archived by the owner on Dec 30, 2021. It is now read-only.

Replaced the request module with got and updated the tests to pass. #101

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

KenEucker
Copy link
Collaborator

@kaimallea

I have replaced the request module with the got module. https://www.npmjs.com/package/got

This required updating a test, and now all 28 tests pass. I haven't tested anything outside of my own usage of this package. One thing I would like to see before merging this in is an example that demonstrates uploading so that can be properly tested. If you get a chance to look over the code that's awesome, but either way let me know what you think the next steps could be on your end.

@kaimallea
Copy link
Owner

kaimallea commented Feb 26, 2021

@KenEucker you're amazing, thanks for doing this! Aside from some unimportant formatting, this looks good -- and works good! (I just tested it locally, uploading a few images).

Re: properly testing upload, I agree. I did so many things wrong when I wrote this ~7 years ago; specifically, the uploadFile method (among other things) has too much magic going on inside (globbing, creating readStreams) instead of just doing one thing well (uploading a file). This makes the uploadFile method difficult to test, requiring a bunch of mocking and stubbing.

Having said that, I'll take a stab adding a test for it. I'll merge this, but will hold off on npm-publishing until I get the test in (I know I said I didn't have time, but I want to help unblock this lib, so I have some motivation atm).

@kaimallea kaimallea merged commit dfeae64 into kaimallea:master Feb 26, 2021
@kaimallea
Copy link
Owner

@KenEucker thanks for the motivation!

@KenEucker
Copy link
Collaborator Author

@kaimallea sounds great!

I started writing an example to include uploading which can be used for testing, but I'm excited to see the improvements you are able to make. I know that this project was moving towards TypeScript and a ground-up rewrite, so I appreciate that you're putting some work into the old code.

I'm happy to remain involved, and I have added more code to my fork for my own projects. Let me know if there's anything else I can contribute!

@KenEucker KenEucker deleted the keneucker/v0.3.2 branch March 9, 2021 05:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants