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

Progress Reporting for long running operations #3243

Merged
merged 202 commits into from
May 25, 2017
Merged

Conversation

oakeyc
Copy link
Contributor

@oakeyc oakeyc commented May 8, 2017


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • The PR has modified HISTORY.rst with an appropriate description of the change (see Modifying change log).

Command Guidelines

  • Each command and parameter has a meaningful description.
  • Each new command has a test.

(see Authoring Command Modules)

Copy link
Member

@derekbekoe derekbekoe left a comment

Choose a reason for hiding this comment

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

Please make sure this PR only has changes for progress reporting. I see frequency.json in these changes.

@@ -63,6 +64,7 @@ def __init__(self):
self.config.add_section('Layout')
self.config.set('Help Files', 'command', 'help_dump.json')
self.config.set('Help Files', 'history', 'history.txt')
self.config.set('Help Files', 'frequency', 'frequency.json')
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be here right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I removed it

message = args.get('message', '')

if percent:
percent = percent
Copy link
Member

Choose a reason for hiding this comment

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

This line does nothing right? Should remove if true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right; I just removed it


def update(self):
""" updates the view with the progress """
# TODO APPROPRIATE PROGRESS SENT
Copy link
Member

Choose a reason for hiding this comment

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

Remove todo?

@@ -0,0 +1,23 @@
interactions:
Copy link
Member

Choose a reason for hiding this comment

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

Was this added accidently?
Can you remove.

@oakeyc oakeyc merged commit 7ee6730 into Azure:master May 25, 2017
@oakeyc oakeyc deleted the progress branch June 1, 2017 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants