Skip to content

Fix marketplace in CLI #4301

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

Merged
merged 1 commit into from
Oct 4, 2021
Merged

Fix marketplace in CLI #4301

merged 1 commit into from
Oct 4, 2021

Conversation

GirlBossRush
Copy link
Contributor

This PR resolves an issue where our marketplace modifications are not applied to the CLI.

  • Also fixes error logging from child process.

- Fix error logging from child process.
@GirlBossRush GirlBossRush added the bug Something isn't working label Oct 3, 2021
@GirlBossRush GirlBossRush requested a review from jsjoeio October 3, 2021 22:50
@GirlBossRush GirlBossRush self-assigned this Oct 3, 2021
@GirlBossRush GirlBossRush requested a review from a team as a code owner October 3, 2021 22:50
@github-actions
Copy link

github-actions bot commented Oct 3, 2021

✨ Coder.com for PR #4301 deployed! It will be updated on every commit.

@GirlBossRush GirlBossRush enabled auto-merge (rebase) October 3, 2021 23:30
@@ -22,8 +22,8 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => {

try {
await cliProcessMain(args)
} catch (error) {
logger.error("Got error from VS Code", field("error", error))
} catch (error: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, are there root error types in Node or TypeScript that we can use instead of any? seems unfortunate not to have any type information

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's traditionally been any unfortunately. But in TypeScript 4.0, they allow you to type it as unknown which they say is safer. But yeah, it's too bad they don't tell us which error this is.

@@ -22,8 +22,8 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => {

try {
await cliProcessMain(args)
} catch (error) {
logger.error("Got error from VS Code", field("error", error))
} catch (error: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's traditionally been any unfortunately. But in TypeScript 4.0, they allow you to type it as unknown which they say is safer. But yeah, it's too bad they don't tell us which error this is.

@GirlBossRush GirlBossRush merged commit 672038c into main Oct 4, 2021
@GirlBossRush GirlBossRush deleted the teffen-marketplace-fix branch October 4, 2021 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants