Skip to content

Use node-fetch for fetchJSONFromURL#11

Merged
dscho merged 3 commits intogit-for-windows:mainfrom
dennisameling:node-fetch
Feb 18, 2021
Merged

Use node-fetch for fetchJSONFromURL#11
dscho merged 3 commits intogit-for-windows:mainfrom
dennisameling:node-fetch

Conversation

@dennisameling
Copy link
Contributor

As requested in git-for-windows/git#3040 (comment):

I have a feeling that this is hodgepodge code, and could be done much more elegantly

Actually, using node-fetch is a lot easier in this case. It makes mocking much easier as well. It's a very lightweight package that also adds convenience methods like res.json() to parse JSON responses.

@dscho
Copy link
Member

dscho commented Feb 18, 2021

Actually, using node-fetch is a lot easier in this case

Ironically, I did use it originally. But I was wary of adding yet more stuff that I do not really need.

Could you add an npm run build && npm run package commit, and compare the output size?

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@dscho dscho merged commit f21a50c into git-for-windows:main Feb 18, 2021
@dscho
Copy link
Member

dscho commented Feb 18, 2021

I only reordered the commits so that dist/ is once again updated only in the end, in a dedicated commit.

@dscho
Copy link
Member

dscho commented Feb 18, 2021

Oh, completely forgot: thank you so much!

@dennisameling dennisameling deleted the node-fetch branch February 18, 2021 12:19
@dscho
Copy link
Member

dscho commented Feb 18, 2021

@dennisameling look at this beauty: https://github.com/marketplace/actions/setup-git-for-windows-sdk

@dennisameling
Copy link
Contributor Author

Love it!! 🚀 🎉

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.

2 participants