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

[Tech debt]: Re-enable telemetry for windows #5901

Closed
dac09 opened this issue Jul 11, 2022 · 4 comments · Fixed by #7389
Closed

[Tech debt]: Re-enable telemetry for windows #5901

dac09 opened this issue Jul 11, 2022 · 4 comments · Fixed by #7389
Labels
bug/confirmed We have confirmed this is a bug

Comments

@dac09
Copy link
Collaborator

dac09 commented Jul 11, 2022

What's not working?

We are disabling telemetry on Windows in #5899 - due to the terminal windows opening when using the CLI.

We need to renable telemetry, once we can workout a way of preventing the terminal windows spawining.

Related Issues

@dac09 dac09 added bug/needs-info More information is needed for reproduction action/add-to-cycle and removed bug/needs-info More information is needed for reproduction labels Jul 11, 2022
@dac09 dac09 added the bug/confirmed We have confirmed this is a bug label Jul 11, 2022
@Gresliebear
Copy link

Gresliebear commented Jul 13, 2022

Node added a windowHide option

nodejs/node#15380

child_process.spawn(cmd, args, { shell: true, detached: true, windowsHide: true })

so some how in dry_run.ts we need to pass 'windowsHide:true' opts on spawnCancellable

image

However I have read that its "On Windows, however, .bat and .cmd files are not executable on their own without a terminal" from the Node docs

I gave it a shot but it didnt work
image

@jtoar
Copy link
Contributor

jtoar commented Jul 14, 2022

@Gresliebear thanks for looking into this. We've tried windowsHide (#5734), but it doesn't seem to work with detached processes: nodejs/node#21825.

@Gresliebear
Copy link

@Gresliebear thanks for looking into this. We've tried windowsHide (#5734), but it doesn't seem to work with detached processes: nodejs/node#21825.

Good Lord it looks like it has not been fixed since 2018

@dac09
Copy link
Collaborator Author

dac09 commented Jul 14, 2022

Good Lord it looks like it has not been fixed since 2018

Haha hears windows XP shutdown sound.

I think what we'll need to do is have a simple reproduction (without our CLI) - when I tried this, I couldn't get the windows to spawn... so I'm confused exactly what the problem is. Maybe it happens when you spawn in certain situations (e.g. through execa/yargs/something we use).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants