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

Migrate the first half of the async demos #46

Merged

Conversation

peter-csala
Copy link
Contributor

Description

  • Migrated demos form 00 to 05 under Async folder
  • Renamed demo 00 to reflect V8's concepts
  • Moved statistics fields from SyncDemo to DemoBase

Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

Just a few minor comments that repeat across a number of files.

PollyDemos/Async/AsyncDemo00_NoStrategy.cs Outdated Show resolved Hide resolved
PollyDemos/Async/AsyncDemo02_WaitAndRetryNTimes.cs Outdated Show resolved Hide resolved
PollyDemos/Async/AsyncDemo02_WaitAndRetryNTimes.cs Outdated Show resolved Hide resolved
PollyDemos/Async/AsyncDemo04_WaitAndRetryForever.cs Outdated Show resolved Hide resolved
Comment on lines +7 to +10
protected int totalRequests;
protected int eventualSuccesses;
protected int eventualFailures;
protected int retries;
Copy link
Member

Choose a reason for hiding this comment

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

Let's make these properties with PascalCase naming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't mind I would do that renaming as a separate PR after all sync and async demos under the Polly.Demos are migrated.

@martincostello martincostello merged commit fe72f44 into App-vNext:main Oct 2, 2023
6 checks passed
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