-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Fix the ARM cmdlet to show the progress bar for long running operation #2941
Conversation
Hi @gandhiniraj, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
TTYL, AZPRBOT; |
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.
Couple of minor nits
@@ -332,26 +332,42 @@ internal ProgressTracker(string activityName, Func<ResourceManagerRestRestClient | |||
this.ProgressRecord = new ProgressRecord(activityId: 0, activity: activityName, statusDescription: "Starting - 0.00% completed."); | |||
} | |||
|
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.
Please include ///summary
/// <summary> | ||
/// Logs the fact that the operation has progressed. | ||
/// </summary> | ||
/// <param name="result">The operation result</param> | ||
internal void UpdateProgress(TrackingOperationResult result) | ||
internal void UpdateProgress(TrackingOperationResult result, bool isResourceCreateOrUpdate) |
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.
I would change isResourceCreateOrUpdate to something more generic, like isLongRunning. This is because we need the progress for not just create or update, but for other operations like resource move
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.
Currently we have two categories for progress bar - if its Create/Update or everything else, hence IMHO this is sufficient
@gandhiniraj can you add missing summary. |
@gandhiniraj would you mind pulling from dev to get the latest changes that fix the on-demand build? Also, please respond to the other review comment made by @vivsriaus |
No description provided.