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

fix: replace request with got for HTTP calls #289

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

cristiand391
Copy link
Member

What does this PR do?

replace request dependency with got.
request is deprecated: https://www.npmjs.com/package/request

What issues does this PR fix or reference?

@W-0@

@@ -2,6 +2,7 @@
"extends": "@salesforce/dev-config/tsconfig",
"compilerOptions": {
"outDir": "lib",
"allowSyntheticDefaultImports": true,
Copy link
Member Author

Choose a reason for hiding this comment

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

wasn't able to build the plugin without this 🤷🏼
TooTallNate/node-agent-base#56

throw new Error(`Unexpected test url - ${url}`);
}
};

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this UT was from when plugin-trust was doing raw HTTP calls to npm, we switched to npm here:
#154

so no need to stub got, the error is thrown from here:

const err = new SfError(error, 'ShellParseError');

@@ -509,109 +546,6 @@ describe('InstallationVerification Tests', () => {
});
});

it.skip('server error', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

ditto, this UT was testing the HTTP calls to npm, we run npm show now (see prev comment)

@mshanemc
Copy link
Contributor

mshanemc commented Aug 5, 2022

QA notes:

linked plugin-trust
✅ install a signed plugin (rel-mgmt), no warning
✅ install a signed plugin (org), no warning (did this because rel-mgmt installs its own plugin-trust, undoing my link, so I re-linked plugin-trust first)
✅ install an unsigned plugin (mine), got warning

not tested: proxies
not tested: a signed plugin with an invalid cert

@peternhale peternhale merged commit 14e75a5 into main Aug 5, 2022
@peternhale peternhale deleted the cd/replace-request-dep branch August 5, 2022 14:14
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