-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: Add Retry Addon documentation #3330
Conversation
WalkthroughThis pull request modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant M as Main Function
participant EB as ExponentialBackoff
participant NR as Network Request
M->>EB: Initialize ExponentialBackoff
loop Retry Attempts
M->>NR: Execute Network Request
NR-->>M: Return response or error
alt Failure (error or non-200)
M->>EB: Wait using exponential backoff
else Success (status 200)
M-->>EB: Proceed with successful response
end
end
M->>M: Panic if all retries fail
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3330 +/- ##
=======================================
Coverage 84.21% 84.21%
=======================================
Files 116 116
Lines 11577 11577
=======================================
Hits 9749 9749
Misses 1397 1397
Partials 431 431
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
addon/retry/README.md (2)
10-17
: 🛠️ Refactor suggestionUpdate TOC Link Fragment Consistency.
The Table of Contents includes the link Default Config Example, but no section exists with that exact anchor. To resolve the markdownlint error (MD051) and avoid confusing readers, please either update the link to match an existing heading (for example, Default Config) or rename the corresponding section heading to "Default Config Example."🧰 Tools
🪛 LanguageTool
[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...(ENGLISH_WORD_REPEAT_RULE)
🪛 GitHub Check: markdownlint
[failure] 17-17: Link fragments should be valid
addon/retry/README.md:17:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md🪛 GitHub Actions: markdownlint
[error] 17-17: MD051/link-fragments Link fragments should be valid [Context: 'Default Config Example']
116-127
: 🛠️ Refactor suggestionAlign Section Heading with TOC.
The section that declares the default configuration variable is titled "## Default Config," yet the Table of Contents lists a link for Default Config Example. For consistency and to fix the markdownlint error, please rename the section header (e.g., to "## Default Config Example") or adjust the TOC accordingly.
🧹 Nitpick comments (2)
addon/retry/README.md (1)
27-63
: Clarify Example Code Variable Naming.
The example in themain
function neatly illustrates how to useretry.NewExponentialBackoff
. However, note that the local variable namedclient
(declared inside the retry callback) shares its name with the imported package. To avoid any potential shadowing or confusion, consider renaming the local variable (for instance, toc
orhttpClient
).docs/addon/retry.md (1)
29-67
: Refine Example Code for Consistency.
The provided example effectively demonstrates the usage of the retry addon. Similar to the README, the callback declares a local variable namedclient
that might conflict with the imported package identifier. Consider renaming this local variable to improve clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/addon/_category_.json
is excluded by!**/*.json
docs/client/_category_.json
is excluded by!**/*.json
docs/extra/_category_.json
is excluded by!**/*.json
docs/guide/_category_.json
is excluded by!**/*.json
📒 Files selected for processing (2)
addon/retry/README.md
(2 hunks)docs/addon/retry.md
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: markdownlint
addon/retry/README.md
[error] 17-17: MD051/link-fragments Link fragments should be valid [Context: 'Default Config Example']
🪛 LanguageTool
docs/addon/retry.md
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...
(ENGLISH_WORD_REPEAT_RULE)
🪛 GitHub Check: markdownlint
docs/addon/retry.md
[failure] 21-21: Link fragments should be valid
docs/addon/retry.md:21:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, macos-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: Compare
- GitHub Check: Analyse
- GitHub Check: repeated
## Table of Contents | ||
|
||
- [Retry Addon](#retry-addon) | ||
- [Table of Contents](#table-of-contents) | ||
- [Signatures](#signatures) | ||
- [Examples](#examples) | ||
- [Default Config](#default-config) | ||
- [Custom Config](#custom-config) | ||
- [Config](#config) | ||
- [Default Config Example](#default-config-example) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ensure Valid TOC Links.
Within the Table of Contents, the link Default Config Example does not match any section heading in the document. To improve navigation and address markdownlint MD051, please update the link so that it corresponds to an existing anchor (for example, matching a header renamed to "## Default Config Example") or modify the TOC entry accordingly.
🧰 Tools
🪛 LanguageTool
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...
(ENGLISH_WORD_REPEAT_RULE)
🪛 GitHub Check: markdownlint
[failure] 21-21: Link fragments should be valid
docs/addon/retry.md:21:3 MD051/link-fragments Link fragments should be valid [Context: "Default Config Example"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md
docs/addon/retry.md
Outdated
## Default Config | ||
|
||
```go | ||
// DefaultConfig is the default config for retry. | ||
var DefaultConfig = Config{ | ||
InitialInterval: 1 * time.Second, | ||
MaxBackoffTime: 32 * time.Second, | ||
Multiplier: 2.0, | ||
MaxRetryCount: 10, | ||
currentInterval: 1 * time.Second, | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consistent Section Naming Recommended.
The section header for the default configuration variable is labeled "## Default Config" but is referenced in the TOC as Default Config Example. Renaming this header to "## Default Config Example" would ensure uniformity across the document and resolve the link fragment issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
addon/retry/README.md (1)
1-7
: Improve Description Clarity in the Introduction
The introductory paragraph explains the addon’s purpose well, but the sentence “If all calls are failed, then, it returns an error.” can be rephrased for clarity. Consider updating it to:-If all calls are failed, then, it returns an error. +If all calls fail, an error is returned.This change improves readability and grammatical accuracy.
docs/addon/retry.md (1)
1-11
: Enhance Introduction Wording in YAML Documentation
The YAML front matter and introductory section effectively introduce the Retry Addon. However, similar to the README, consider rephrasing the sentence in the introduction to avoid repetition and improve clarity. For example:-If all calls are failed, then, it returns an error. It adds a jitter at each retry step because adding - a jitter is a way to break synchronization across the client and avoid collision. +If all calls fail, an error is returned. A jitter is added at each retry step to break client synchronization and avoid collisions.This revision removes redundant wording and makes the description more concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
addon/retry/README.md
(2 hunks)docs/addon/retry.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/addon/retry.md
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, macos-13)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: Compare
- GitHub Check: repeated
🔇 Additional comments (5)
addon/retry/README.md (2)
8-18
: Table of Contents Validation
The Table of Contents accurately lists all key sections, including “Default Config Example.” Ensure that the corresponding section header exactly matches the TOC entry so that in-page navigation works correctly. Currently, the TOC entry “Default Config Example” aligns with the later header, so no changes are necessary here.🧰 Tools
🪛 LanguageTool
[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...(ENGLISH_WORD_REPEAT_RULE)
25-63
: Examples Code Block – Clear and Comprehensive
The complete example in the “Examples” section demonstrates how to initialize the exponential backoff and perform a network request with retry logic. The inline comments make the code easy to follow. One consideration: while usingpanic(err)
is acceptable in a demonstrative context, you might want to note (in comments) that in production code more graceful error handling could be preferable.docs/addon/retry.md (3)
12-22
: Table of Contents Consistency
The TOC accurately reflects the document’s sections, and the link “Default Config Example” now matches the header “## Default Config Example.” This resolves previous navigation issues and aligns with best practices for markdown links.🧰 Tools
🪛 LanguageTool
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...(ENGLISH_WORD_REPEAT_RULE)
29-67
: Examples Code Block – Clear Demonstration
The “Examples” section provides a full, illustrative usage example of the retry functionality. The sample code is clear, well-commented, and effectively demonstrates how to handle errors during retries using the exponential backoff mechanism.
69-128
: Configuration and Default Config Example Sections are Well Structured
The “Default Config,” “Custom Config,” “Config,” and “Default Config Example” sections comprehensively document the addon's configuration options. The code examples are consistent and maintain a clear style throughout the document. These sections offer valuable, easily digestible information for users looking to customize their retry behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- pls add the addon to the https://docs.gofiber.io/next/whats_new markdown
@grivera64 can you check our hints |
@ReneWerner87 For sure, I've made the changes based on the hints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
addon/retry/README.md (2)
27-62
: Example Main Function – Avoid Variable Shadowing
The example code clearly demonstrates how to initialize the exponential backoff and use the retry mechanism. However, in the anonymous function (line 45), the local variableclient
is declared usingclient := client.New()
, which shadows the imported package namedclient
. This shadowing can lead to confusion and potential maintenance issues. Consider renaming the local variable (for example, tohttpClient
) to improve clarity.- client := client.New() + httpClient := client.New()
84-92
: Default Config Example – Document Only Public Fields
The "Default Config Example" section provides a clear default configuration; however, the fieldcurrentInterval
(line 90) appears to be an internal state used for tracking purposes and is not exported (note the lowercase first letter). If this field is not intended for external configuration, consider omitting it from the public documentation to avoid confusion for users.docs/addon/retry.md (2)
31-66
: Example Main Function – Avoid Variable Shadowing
Similar to the README file, the example main function here (lines 31–66) is effective in demonstrating the usage of the retry addon. However, note the same issue on line 49 whereclient := client.New()
is used. Renaming this local variable (for example, tohttpClient
) would avoid shadowing the imported package name and clarify the intent.- client := client.New() + httpClient := client.New()
85-96
: Default Config Example – Expose Only Public Configuration
In the "Default Config Example" section, theDefaultConfig
variable is clearly defined. However, the inclusion of the fieldcurrentInterval
(line 94) might be confusing if it is considered internal state. Since it is not exported (due to the lowercase first letter), consider whether it should be documented for public use. If it is not intended for users to modify or rely on, you might remove it from the example or add a clarifying note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/addon/_category_.json
is excluded by!**/*.json
📒 Files selected for processing (2)
addon/retry/README.md
(1 hunks)docs/addon/retry.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, macos-13)
- GitHub Check: unit (1.23.x, macos-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: Compare
- GitHub Check: Analyse
- GitHub Check: repeated
🔇 Additional comments (2)
docs/addon/retry.md (2)
1-3
: Front Matter Verification
The front matter (lines 1–3) is present and properly formatted for identifying the document. No changes needed.
12-21
: Table of Contents – Verify Anchor Consistency
The Table of Contents lists several section links (including "Default Config Example"). Make sure that each link exactly matches the corresponding header (e.g. the header at line 85 is "## Default Config Example") so that users can navigate the document correctly.
d280200
to
842a05b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
addon/retry/README.md (1)
111-123
:⚠️ Potential issueInconsistent DefaultConfig Definition:
TheDefaultConfig
variable includes an unrecognized fieldcurrentInterval
, which is not defined in theConfig
struct above. This inconsistency may lead to confusion or compile errors. Consider removing thecurrentInterval
field to align with the struct definition.A suggested diff:
var DefaultConfig = Config{ InitialInterval: 1 * time.Second, MaxBackoffTime: 32 * time.Second, Multiplier: 2.0, MaxRetryCount: 10, - currentInterval: 1 * time.Second, }
docs/addon/retry.md (1)
115-127
:⚠️ Potential issueInconsistent DefaultConfig Definition:
Similar to the README file, theDefaultConfig
variable here includes thecurrentInterval
field, which is not part of the definedConfig
struct. Removing this field will prevent potential confusion and ensure consistency between the struct and its default initialization.A suggested diff:
var DefaultConfig = Config{ InitialInterval: 1 * time.Second, MaxBackoffTime: 32 * time.Second, Multiplier: 2.0, MaxRetryCount: 10, - currentInterval: 1 * time.Second, }
🧹 Nitpick comments (2)
addon/retry/README.md (1)
8-17
: Ensure Valid TOC Links:
The Table of Contents is comprehensive. Please verify that every anchor link (e.g., “#default-config”, “#default-config-example”) exactly matches its corresponding section header. Also, consider whether the separation between "Default Config" (usage example) and "Default Config Example" (detailed variable declaration) is clear and necessary.🧰 Tools
🪛 LanguageTool
[duplication] ~15-~15: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...(ENGLISH_WORD_REPEAT_RULE)
docs/addon/retry.md (1)
12-22
: Valid TOC and Section Consistency:
The Table of Contents lists all the key sections. Ensure that each TOC link correctly corresponds to the section headers and consider clarifying the distinction between “Default Config” (a simple usage example) and “Default Config Example” (the detailed default variable declaration).🧰 Tools
🪛 LanguageTool
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/addon/_category_.json
is excluded by!**/*.json
📒 Files selected for processing (2)
addon/retry/README.md
(2 hunks)docs/addon/retry.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/addon/retry.md
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (13)
addon/retry/README.md (6)
1-7
: Clear Introduction:
The introduction succinctly explains what the Retry Addon does and its intended mechanism using exponential backoff with jitter. This sets the right context for users.
19-23
: Clear API Signature:
The function signature forNewExponentialBackoff
is presented clearly, making it easy for users to understand how to initialize the addon.
25-63
: Effective Usage Example:
Themain
function example demonstrates the retry mechanism very well, including proper error handling and retry logic. The overall flow is easy to follow.
65-69
: Default Configuration Call:
The simple callretry.NewExponentialBackoff()
effectively shows how the addon is used with default configuration settings.
71-80
: Custom Config Usage:
The custom configuration example clearly demonstrates how to override default parameters, providing explicit values for timing, multiplier, and retry count.
82-110
: Comprehensive Config Struct:
TheConfig
struct is well documented with each field’s description and default value, which is very helpful for users seeking to customize their configuration.docs/addon/retry.md (7)
1-4
: Front Matter is Correct:
The YAML front matter (includingid: retry
) is properly set up for this documentation page.
5-11
: Clear Introduction:
The header and introductory text clearly explain the purpose and functionality of the Retry Addon, making its role and behavior easy to understand.
23-28
: Clear Function Signature:
TheNewExponentialBackoff
function signature is displayed clearly within a code block, effectively communicating the API.
29-67
: Effective Retry Example:
The example provided in themain
function clearly demonstrates how to use the retry mechanism, including proper error handling and output messaging.
69-73
: Default Config Usage Example:
The snippet that callsretry.NewExponentialBackoff()
with no arguments succinctly shows the default behavior of the addon.
75-84
: Custom Config Example:
The custom configuration example is well-crafted and illustrates how to override default parameters effectively.
86-113
: Well-Documented Config Struct:
TheConfig
struct is thoroughly detailed with explanations for each field and their default values, aiding user comprehension of the available configuration options.
|
Done, sorry about missing those hints the first time around. I've added a commit to add retry to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
addon/retry/README.md (1)
115-122
:⚠️ Potential issueCritical Issue: Remove Deprecated Field from Default Config
The "Default Config Example" block (lines 115–122) still includes a value forcurrentInterval
, which has been removed from theConfig
struct. This leads to inconsistency and could confuse users. Please remove the line settingcurrentInterval
so that the default configuration only assigns the documented fields. For example:var DefaultConfig = Config{ - InitialInterval: 1 * time.Second, - MaxBackoffTime: 32 * time.Second, - Multiplier: 2.0, - MaxRetryCount: 10, - currentInterval: 1 * time.Second, + InitialInterval: 1 * time.Second, + MaxBackoffTime: 32 * time.Second, + Multiplier: 2.0, + MaxRetryCount: 10, }docs/addon/retry.md (1)
117-127
:⚠️ Potential issueCritical Issue: Remove Deprecated Field from Default Config Example
Similar to the issue inaddon/retry/README.md
, the "Default Config Example" in this document (lines 117–127) includes thecurrentInterval
field. Please remove it so that the DefaultConfig instance only includes the fields defined in theConfig
struct. For example:var DefaultConfig = Config{ - InitialInterval: 1 * time.Second, - MaxBackoffTime: 32 * time.Second, - Multiplier: 2.0, - MaxRetryCount: 10, - currentInterval: 1 * time.Second, + InitialInterval: 1 * time.Second, + MaxBackoffTime: 32 * time.Second, + Multiplier: 2.0, + MaxRetryCount: 10, }This adjustment will ensure consistency with the updated
Config
struct.
🧹 Nitpick comments (3)
docs/whats_new.md (2)
36-37
: TOC Update: Verify Anchor Link Validity
The addition of the "🔌 Addons" entry in the Table of Contents (line 36) is clear. However, please verify that the anchor "#-addons
" (if that is generated from the header) correctly corresponds to the "## 🔌 Addons" section later in the document. Also, note the pipeline error regarding the "📋 Migration guide" link at line 37; ensure that the anchor in that link exactly matches the corresponding header.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
37-37: Link fragments should be valid
null(MD051, link-fragments)
🪛 GitHub Check: markdownlint
[failure] 37-37: Link fragments should be valid
docs/whats_new.md:37:3 MD051/link-fragments Link fragments should be valid [Context: "📋 Migration guide"] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md051.md🪛 GitHub Actions: markdownlint
[error] 37-37: MD051/link-fragments Link fragments should be valid [Context: '📋 Migration guide']
943-995
: Content Clarity: Refine Retry Addon Description
The new "Retry" addon section (lines 943–995) provides a detailed explanation and usage example, which is great. Consider refining some language for clarity. For instance, rephrase “calls the function multiple times and tries to make it successful. If all calls are failed, then, it returns an error” to “calls the function repeatedly until it succeeds; if all attempts fail, it returns an error.” This small change improves readability without altering the technical details.addon/retry/README.md (1)
68-84
: Example Code Review: Clarity and Completeness
The examples provided for both default and custom configurations are clear and practical. Ensure that users know to import"time"
when using these examples, as time-based constants are used in the configuration. This is a minor documentation detail that could prevent confusion for new users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
addon/retry/README.md
(2 hunks)docs/addon/retry.md
(1 hunks)docs/whats_new.md
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: markdownlint
docs/whats_new.md
[error] 37-37: MD051/link-fragments Link fragments should be valid [Context: '📋 Migration guide']
🪛 LanguageTool
docs/addon/retry.md
[duplication] ~19-~19: Possible typo: you repeated a word.
Context: ...ault Config](#default-config) - Custom Config - Config - [Default Config Example](#d...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, macos-13)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (3)
addon/retry/README.md (1)
26-27
: Signature Update is Correct
The updated function signaturefunc NewExponentialBackoff(config ...retry.Config) *retry.ExponentialBackoffaccurately reflects the changes described in the PR objectives. Good job updating the documentation accordingly.
docs/addon/retry.md (2)
1-7
: Front Matter and Title: Looks Good
The front matter along with the title and introductory description clearly sets the context for the Retry Addon documentation. The added ID (id: retry
) will help in link generation.
23-28
: TOC and Section Alignment
The Table of Contents in this file is comprehensive. The "Default Config Example" entry matches the section header in line 115, which resolves earlier concerns from previous reviews. Great work aligning the section names.
@grivera64 can you check the lint errors ? |
334095c
to
ada57d2
Compare
Fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/whats_new.md (1)
943-995
: Retry Addon Documentation Clarity and Style
The new "Retry" addon section is well organized and provides a clear example of how to use the feature. To further enhance clarity and readability, consider the following suggestions:
- Grammar Improvement:
Instead of:-It calls the function multiple times and tries to make it successful. If all calls are failed, then, it returns an error. +It repeatedly calls the function until it succeeds; if all retry attempts fail, it returns an error.- Jitter Explanation:
Consider rephrasing the sentence about jitter for clarity. For example,
"It adds a jitter to each retry attempt to prevent clients from synchronizing their retries, thus reducing the risk of collision."Overall, these minor refinements will make the documentation even clearer for users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/whats_new.md
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: Compare
- GitHub Check: repeated
🔇 Additional comments (1)
docs/whats_new.md (1)
36-36
: New Navigation Entry for Addons
Adding the "🔌 Addons" navigation link to the table of contents is a welcome update and is consistent with the structure of the other sections. Please double-check that the anchor (#-addons
) correctly maps to the new "Addons" section below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/whats_new.md (2)
943-952
: New Addons Documentation Section
The documentation for Addons—and specifically for the Retry addon—is comprehensive and well organized. The description explains the purpose of the Retry addon and its basic working mechanism.Suggestions:
- Consider refining a couple of phrases for clarity. For example, change “tries to make it successful. If all calls are failed, then, it returns an error.” to “tries to succeed, and if all attempts fail, it returns an error.”
- Remove the extraneous comma after “then” for a smoother read.
953-995
: Clear and Practical Retry Example
The example code clearly demonstrates how to use the Retry addon with an exponential backoff. It effectively illustrates the creation of a backoff instance, usage of a retry loop, and error handling upon an unsuccessful network request.Suggestions:
- Avoid Shadowing the Package Name: In the retry callback, the local variable named
client
shadows the imported package name. To improve clarity and avoid potential confusion, consider renaming this local variable. For example:- err = expBackoff.Retry(func() error { - client := client.New() - resp, err = client.Get("https://gofiber.io") + err = expBackoff.Retry(func() error { + httpClient := client.New() + resp, err = httpClient.Get("https://gofiber.io")This change will prevent the inadvertent shadowing of the package name
client
and enhance code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/whats_new.md
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: Compare
- GitHub Check: repeated
🔇 Additional comments (1)
docs/whats_new.md (1)
36-38
: New TOC Entry for Addons
The addition of the "🔌 Addons" entry to the table of contents nicely reflects the newly introduced section below. This update improves navigability and ensures that users can quickly find information on the new Retry addon.
Description
Fiber added its first add-on in PR #1972, called retry. The documentation for this add-on is not currently included in v3's documentation on docs.gofiber.io.
This PR adds this missing documentation under a new collapsible tab under the middleware tab.
Fixes #3326
Changes introduced
retry
Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/
directory for Fiber's documentation.