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: verify fails if node not installed #167

Merged
merged 20 commits into from
Sep 24, 2021
Merged

fix: verify fails if node not installed #167

merged 20 commits into from
Sep 24, 2021

Conversation

peternhale
Copy link
Contributor

@peternhale peternhale commented Sep 22, 2021

@W-9930707@

What does this PR do?

What issues does this PR fix or reference?

forcedotcom/cli#1198

RodEsp
RodEsp previously requested changes Sep 22, 2021
src/commands/plugins/trust/verify.ts Outdated Show resolved Hide resolved
src/commands/plugins/trust/verify.ts Outdated Show resolved Hide resolved
@maggiben maggiben requested a review from RodEsp September 23, 2021 02:08
src/lib/npmCommand.ts Outdated Show resolved Hide resolved
src/lib/npmCommand.ts Outdated Show resolved Hide resolved
src/lib/npmCommand.ts Outdated Show resolved Hide resolved
maggiben and others added 2 commits September 23, 2021 17:50
Co-authored-by: Shane McLaughlin <[email protected]>
Co-authored-by: Shane McLaughlin <[email protected]>
@RodEsp RodEsp dismissed their stale review September 24, 2021 00:17

Requests were addressed

@@ -6,9 +6,12 @@
*/
/* eslint-disable @typescript-eslint/no-unused-vars */

import { type as osType } from 'os';
Copy link
Contributor Author

@peternhale peternhale Sep 24, 2021

Choose a reason for hiding this comment

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

@maggiben why the alias? This confuses old people like me :) I would rather see os.type()

Copy link
Contributor

@RodEsp RodEsp Sep 24, 2021

Choose a reason for hiding this comment

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

@peternhale that was me.
Because if you want to do os.type() then you have to bring in all of os with import * as os from 'os';.
So you can do import { type } from 'os'; to only bring in what you need but then calling type() isn't very descriptive either, so I aliased it to osType();

@mshanemc mshanemc merged commit 0393b90 into main Sep 24, 2021
@mshanemc mshanemc deleted the phale/W-9930707 branch September 24, 2021 20:15
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.

4 participants