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

Remove dependency on "request" package #149

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

2colours
Copy link
Contributor

@2colours 2colours commented Jan 7, 2025

It was only used in the download-node script - now that also uses superagent instead.

I tested the download locally and it did succeed.

2colours and others added 2 commits January 7, 2025 02:20
It was only used in the download-node script - now that also uses superagent instead.
Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

The diff here looks to have identical meaning. And I've tested to ensure NodeJS is still being installed successfully, and contains the same amount of data between installs.

Great work on this final user of request, I'll be happy to see us finally get rid of it!

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. 👍

CI is passing, with a successful rebuild of ppm's own dependencies against the downloaded Node, so the download script must be working fine. Works for me locally as well.

With two Approves, I don't see why not to merge, so I'll go ahead and merge this.

Thanks again.

@DeeDeeG DeeDeeG merged commit 6981ce7 into pulsar-edit:master Feb 14, 2025
11 checks passed
@2colours 2colours deleted the remove-request-dependency branch February 16, 2025 14:34
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.

3 participants