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

Inherit stdio in launch + make browsers optional for update-browsers #273

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

Domiii
Copy link
Contributor

@Domiii Domiii commented Nov 12, 2023

  1. Inherit stdio in launch, s.t. those who want to debug it can (else RECORD_REPLAY_VERBOSE is sent to Nirvana).
    • Also, on my Linux machine, not inheriting stdio seems to deadlock the browsers. I tried to debug by enabling it, but that just fixes it...
    • Either way, it would be good to have an option for the user to have any chance to get debugging data out and forward it to us, if they are getting stuck 🤷‍♀️
  2. Don't unref from the child process when attach is enabled, and instead wait for it to finish. This way, the replay launch process stays in sync with the lifecycle of the runtime, which allows any observer or caller to more easily wrap it.
  3. Make browsers optional for update-browsers, because that's what it was before and its easier to use for the average lazy person (like myself 👀)

@Domiii Domiii requested a review from ryanjduffy November 12, 2023 08:02
@Domiii Domiii self-assigned this Nov 12, 2023
@@ -109,7 +109,7 @@ commandWithGlobalOptions("rm-all")

commandWithGlobalOptions("update-browsers")
.description("Update browsers used in automation.")
.arguments("<browsers...>")
.arguments("[<browsers...>]")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a valid change but please don't add put unrelated changes in the same PRs

@Domiii Domiii merged commit 917cf21 into main Nov 15, 2023
2 of 4 checks passed
@Domiii Domiii deleted the pass-stdio-to-launch branch November 15, 2023 08:48
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