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

feat: add unpacking indicator to the install command #13356

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Abhash-Chakraborty
Copy link

@Abhash-Chakraborty Abhash-Chakraborty commented Dec 3, 2024

Summary of Updates in CLI.ts:

Enhanced error handling and logging for improved user feedback during CLI operations.
Streamlined argument parsing for clarity and efficiency.
Added debug messages for tracking command execution and potential failures.
Optimized the CLI workflow to handle edge cases and ensure robust functionality.

Bug: #13083

Copy link

google-cla bot commented Dec 3, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Lightning00Blade Lightning00Blade changed the title feat: add unpacking indicator to the install command(#13083) feat: add unpacking indicator to the install command Dec 4, 2024
if (downloadedBytes >= totalBytes && !isExtracting) {
// Transition to the extraction phase
progressBar.terminate(); // Clear download progress bar
console.log(`Extracting ${browser} ${buildId}...`); // Simple console message for now
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this would report extracting even when no extracting is happening (options.unpack === false) and also it would not cover the waiting for running setup and installing deps (options.installDeps === true).

We probably also should not use console.log directly as the progress bar is only rendering if the output stream isTTY.

I think using a static message is fine but we should also write it to process.stderr as the progress bar and we can add messages directly to the corresponding code that does extraction/deps/setup conditions on the presence of downloadProgressBar that indicates whether the code is invoked from CLI.

@github-actions github-actions bot added the Stale label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants