这是indexloc提供的服务,不要输入任何密码
Skip to content

output to terminal when we are just waiting for cache uploads #1686

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 5 commits into from
Aug 15, 2022

Conversation

gsoltis
Copy link
Contributor

@gsoltis gsoltis commented Aug 11, 2022

As a stopgap before we move cache uploads to the daemon, I added some terminal output if uploading is blocking process completion.

While obviously not ideal that users are just waiting for cache population, I think it's at least better that they know that that is what is happening, rather than the process appearing to hang.

@vercel
Copy link

vercel bot commented Aug 11, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
turbo-site ✅ Ready (Inspect) Visit Preview Aug 15, 2022 at 5:16PM (UTC)

@gsoltis
Copy link
Contributor Author

gsoltis commented Aug 11, 2022

We could possibly improve the UX here with something like a progress bar or spinner

@gsoltis
Copy link
Contributor Author

gsoltis commented Aug 12, 2022

Added a spinner. Here's what it looks like (only triggers after 1.5s of waiting)
spinner
.

@gsoltis gsoltis force-pushed the gsoltis/notify_upload branch from 90566b8 to 707ed74 Compare August 12, 2022 23:33
@gsoltis gsoltis marked this pull request as ready for review August 12, 2022 23:45
Copy link
Member

@tknickman tknickman left a comment

Choose a reason for hiding this comment

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

Looks good - one comment for a ...comment 😄

I like the approach - hopefully we won't need this anymore once we can move it to the daemon - but this is definitely a better ux for now.

}

// WaitFor runs fn, and prints msg to the terminal if it takes longer
// than initialDay to do complete. Depending on the terminal configuration, it may
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// than initialDay to do complete. Depending on the terminal configuration, it may
// than initialDelay to complete. Depending on the terminal configuration, it may

Copy link
Member

Choose a reason for hiding this comment

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

Kodiak beat me here - addressed in #1697

@kodiakhq kodiakhq bot merged commit 1cf9024 into main Aug 15, 2022
@kodiakhq kodiakhq bot deleted the gsoltis/notify_upload branch August 15, 2022 17:26
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.

2 participants