Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1767 +/- ##
=======================================
Coverage 84.53% 84.53%
=======================================
Files 307 307
Lines 6777 6777
Branches 1043 1043
=======================================
Hits 5729 5729
Misses 839 839
Partials 209 209
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
|
||
| ## Anti-patterns | ||
|
|
||
| ### Ignoring Cancellation Token |
There was a problem hiding this comment.
On other pages the anti-patterns are numbered. I think it might make sense to add numbering here as well
| ### Ignoring Cancellation Token | |
| ### 1 - Ignoring Cancellation Token |
There was a problem hiding this comment.
Does the number add any value though?
There was a problem hiding this comment.
If you have multiple paragraphs and one might refer to a previous one then its number could enough.
But I also did not follow this :D
- http://localhost:8080/strategies/retry.html#6---having-a-single-strategy-for-multiple-failures
- It is referring to the 5th paragraph via Previously, it was suggested that
There was a problem hiding this comment.
Let me delete all the numbers in my next PR where I change the [!IMPORTANT] blocks to [!INFO]
docs/strategies/timeout.md
Outdated
| .Build(); | ||
|
|
||
| await pipeline.ExecuteAsync( | ||
| async innerToken => await Task.Delay(3000, outerToken), // The delay call should use innerToken |
There was a problem hiding this comment.
In every other places we use TimeSpan.FromXYZ in the examples. I would suggest to use here as well.
| async innerToken => await Task.Delay(3000, outerToken), // The delay call should use innerToken | |
| async innerToken => await Task.Delay(TimeSpan.FromSeconds(3), outerToken), // The delay call should use innerToken |
docs/strategies/timeout.md
Outdated
|
|
||
| **Reasoning**: | ||
|
|
||
| The provided callback respects the `innerToken` provided by the pipeline, and as a result, the callback is correctly cancelled by the timeout strategy after 1 second. |
There was a problem hiding this comment.
| The provided callback respects the `innerToken` provided by the pipeline, and as a result, the callback is correctly cancelled by the timeout strategy after 1 second. | |
| The provided callback respects the `innerToken` provided by the pipeline, and as a result, the callback is correctly cancelled by the timeout strategy after 1 second plus `TimeoutRejectedException` is thrown. |
3 participants
Details on the issue fix or feature implementation
Added more details to timeout strategy and introduced anti-patterns section.
Confirm the following