-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Docs] Revise migration guide 3/3 #1775
[Docs] Revise migration guide 3/3 #1775
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1775 +/- ##
=======================================
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. |
@peter-csala thanks again for a great content! Here is what I think:
I would say so, it's better than not to mention anything.
I don't think state is something we need to mention in migration guide as it is a concept new to v8. There is already briefly mentioned here: But maybe we should expand the resilience pipeline docs with new "Executing pipeline" section where we can describe state too: |
I agree that would be a great place for that. |
Co-authored-by: Martin Costello <[email protected]>
@martintmk Will you create a PR for ☝️ ? |
I won't be able this week, will try get to it next one. You can take a shot at it if you want. |
I am on parental leave in November as my second son was born on Monday. I will have really limited presence here in the next 3-4 weeks. If you won't have time for this during November then I will give it a try in December. |
Congrats Peter and enjoy your parental leave! :) I'll try get to it if possible. |
@martintmk Am I right that you did not have time to file a PR regarding ☝🏻 ? |
Sadly, not really. Haven't had time recently. |
No worries, I'll take care of it.🤘 |
Pull Request
The issue or feature being addressed
Details on the issue fix or feature implementation
Fixed a typo in
resilience-pipeline-registry.md
Fixed the tip of the migration of retry policies
Fixed circuit breaker V8's warning about discontinued support for classic CB
Get rid of the Migrating other policies section because it just echoed previous statements
Migrating `Polly.Context
ResiliencePropertyKey
Migrating safe execution
Migrating no-op policies
T
toTResult
for clarityMigrating policy registries
T
toTKey
for clarityInteroperability between policies and resilience pipeline
Open question(s)
Answer: Do not mention
Migrating safe execution
state
Answer: Extend the resilience pipelines page
Confirm the following