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.

Sign up for GitHub

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

Jump to bottom

Fix sample code for Timeout overload #1109

Merged
merged 1 commit into from
Apr 11, 2023
Merged

Fix sample code for Timeout overload #1109

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ Policy

// Configure variable timeout via a func provider.
Policy
.Timeout(() => myTimeoutProvider)) // Func<TimeSpan> myTimeoutProvider
.Timeout(myTimeoutProvider)) // Func<TimeSpan> myTimeoutProvider
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, it could also be this:

Suggested change
.Timeout(myTimeoutProvider)) // Func<TimeSpan> myTimeoutProvider
.Timeout(() => myTimeoutProvider())) // Func<TimeSpan> myTimeoutProvider

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, but why someone would need to do this? 😊

Copy link
Member

Choose a reason for hiding this comment

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

IMHO it makes the intent clearer to the reader outside of an IDE (i.e. in our documentation) that it's a delegate and not simple variable.

Copy link
Contributor Author

@prodehghan prodehghan Apr 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I also thought of that, but that's why there is a comment in front of the code. It is not always possible (or good) to make everything super easy. The reader already took the time to read the README file to that point and already looked at samples. At this point, there are four different hints for the reader: 1- This sample should have something new compared to the two samples above it, 2- The comment above the code says "via a func provider", 3- the variable name ends with Provider, 4- The comment in front states the variable type.

No one can help someone that just wants to copy a code snippet that he/she found, blindly, without paying attention.

Sorry for the long comment 😋. I hope I was able to deliver my point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the library is already complicated enough, that not paying attention will not help the reader.


// Timeout, calling an action if the action times out
Policy
Expand Down