-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add timing information to commands #2164
Conversation
Maybe we can just have the timing at the end of provision? On each line for each resource seems a little wordy to me. I'd also probably air on the side of just adding it to the SUCCESS statement in parens (e.g. "(took x minutes y seconds)") as opposed to adding a new line of text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I had some initial concerns about the timing outputs matching, but I think if we end up highlighting only a few long-running operations, it doesn't end up mattering. The overall provisioning time is nice.
Overall, it is an improvement and something I think users will appreciate.
cli/azd/pkg/infra/provisioning/provisioning_progress_display.go
Outdated
Show resolved
Hide resolved
I agree with @savannahostrowski's suggestions. Appending the time at the end of each sub-step that is over 5 seconds is a bit confusing, its not clear why it shows for some and not others. I also have concerns about line-length getting too long. For now, lets surface the time on sub-steps only on failure, and at the end like Savannah suggested. SUCCESS: Your project was provisioned in 6 minutes and 16 seconds Edit: If its critical for users to understand the time each sub-step took that was greater than 5 seconds, we can include for now. But I think we'd be much better off revisiting as part of a future UX retro to avoid introducing new experience debt. |
This change adds timing information to the success messages printed by `build`, `deploy`, `down`, `package`, `provision`, `restore` and `up`. Fixes Azure#414
1097321
to
faa0d3f
Compare
azd provision
Pushed an updated copy of the code. Let me know what you think, @weikanglim, @savannahostrowski and @Austinauth. I can take additional screenshots if you'd like, but I think the message above gives a good sense of the changes, since all the commands use the same design language now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small nit around wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great Matt, thank you for making the tweaks. I don't feel super strongly about the language one way or another (both the existing language and Savannah's suggestions), so I'm cool with whatever you and her land on.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference (preview)
|
This change adds timing information to the success messages printed by
build
,deploy
,down
,package
,provision
,restore
andup
.Fixes #414