📚 Doc: Updates to API documentation and README #3205
Conversation
gaby
requested review from
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe changes in this pull request involve extensive updates to the documentation across various files, primarily focusing on improving clarity, consistency, and usability. Key modifications include formatting adjustments, grammatical corrections, and the addition of code examples with enhanced readability. Specific sections were restructured, and terminology was standardized to ensure uniformity. The updates do not alter the underlying functionality of the code but aim to make the documentation more accessible and user-friendly. Changes
Possibly related PRs
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 (
|
|
@coderabbitai ignore |
|
Will fix lint issues in the morning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (21)
docs/api/hooks.md (1)
10-10: Add a comma after the introductory phrase.Add a comma after "With Fiber" to improve readability:
-With Fiber you can execute custom user functions at specific method execution points. +With Fiber, you can execute custom user functions at specific method execution points.🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A comma is probably missing here.
Context: ...rt TabItem from '@theme/TabItem'; With Fiber you can execute custom user functions a...(MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
docs/api/log.md (4)
Line range hint
44-71: Consider adding return values to the examples.The examples are clear and well-formatted. However, it might be helpful to show the expected log output format for each level.
Example enhancement:
log.Info("Hello, World!") +// Output: {"level":"info","msg":"Hello, World!","time":"2024-11-20T10:00:00Z"}
79-83: Consider using descriptive message strings.While the examples correctly demonstrate key-value logging, using empty strings ("") as messages might not be the best practice. Consider using descriptive messages that provide context.
-log.Debugw("", "greeting", "Hello", "target", "boy") +log.Debugw("greeting message", "greeting", "Hello", "target", "boy")
154-174: Consider using more restrictive file permissions.While the error handling is good, the file permissions (0666) allow read-write access for all users. Consider using more restrictive permissions like 0644 for log files unless there's a specific requirement.
-f, err := os.OpenFile("test.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) +f, err := os.OpenFile("test.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
Line range hint
179-184: LGTM! Consider adding a practical use case example.The context binding explanation is clear. Consider adding a practical example showing how context values are included in the logs.
Example addition:
ctx := context.WithValue(context.Background(), "requestID", "123") commonLogger := log.WithContext(ctx) commonLogger.Info("processing request") // Output: {"level":"info","msg":"processing request","requestID":"123","time":"..."}docs/api/redirect.md (5)
9-9: Enhance the introduction with supported redirect typesConsider expanding the introduction to specify the types of redirects supported (e.g., URL-based, route-based, back/referrer-based).
-The redirect package is used to redirect the context (request) to a different URL or route. +The redirect package provides multiple ways to redirect requests, including URL-based redirects, named route redirects, and referrer-based redirects. It supports both internal routes and external URLs.
15-15: Add common status code examplesConsider adding examples of commonly used status codes for redirects to make the documentation more practical.
-Redirects to the URL derived from the specified path, with a specified [status](#status), a positive integer that corresponds to an HTTP status code. +Redirects to the URL derived from the specified path, with a specified [status](#status) code (e.g., 301 for permanent redirects, 302 for temporary redirects, 303 for see other, 307 for temporary redirects that maintain the HTTP method).
Line range hint
92-106: Enhance Back method examplesThe current example could be more illustrative of real-world usage patterns.
app.Get("/", func(c fiber.Ctx) error { return c.SendString("Home page") }) -app.Get("/test", func(c fiber.Ctx) error { +// Example showing referer-based redirect +app.Get("/product", func(c fiber.Ctx) error { c.Set("Content-Type", "text/html") - return c.SendString(`<a href="/back">Back</a>`) + return c.SendString(` + <a href="/cart">Add to Cart</a> + <br> + <a href="/back">Back to previous page</a> + `) }) -app.Get("/back", func(c fiber.Ctx) error { - return c.Redirect().Back("/") +// Example showing fallback when referer is missing +app.Get("/cart", func(c fiber.Ctx) error { + return c.Redirect().Back("/product") // Falls back to /product if no referer })
128-128: Simplify wording for better readabilityThe phrase "in conjunction with" can be simplified.
-It is used in conjunction with [**To**](#to), [**Route**](#route), and [**Back**](#back) methods. +It can be used with the [**To**](#to), [**Route**](#route), and [**Back**](#back) methods. -It is used in conjunction with the [**Route**](#route) method. +It is used with the [**Route**](#route) method.Also applies to: 147-147
🧰 Tools
Line range hint
150-156: Enhance RedirectConfig documentationThe RedirectConfig struct documentation could be more detailed about its fields.
```go title="Definition" -// RedirectConfig is a config to use with Redirect().Route() +// RedirectConfig defines the configuration for route-based redirects type RedirectConfig struct { - Params fiber.Map // Route parameters - Queries map[string]string // Query map + // Params contains route parameters that will replace :param placeholders in the route path + Params fiber.Map + + // Queries defines URL query parameters that will be appended to the redirect URL + Queries map[string]string }🧰 Tools
🪛 LanguageTool
[style] ~147-~147: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...n for the redirect. :::info It is used in conjunction with the Route method. ::: ``...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
docs/api/app.md (4)
82-82: Enhance the caution note with a concrete example.The caution note about mounting order could be more helpful with a specific example demonstrating the potential issues.
Consider expanding the note with a code example:
-Mounting order is important for `MountPath`. To get mount paths properly, you should start mounting from the deepest app. +Mounting order is important for `MountPath`. To get mount paths properly, you should start mounting from the deepest app. For example: + +```go +// ❌ Incorrect order +one.Use("/two", two) +two.Use("/three", three) // MountPath() will be incorrect + +// ✅ Correct order +two.Use("/three", three) +one.Use("/two", two) // MountPath() will be correct +```
93-119: Enhance the Group example with middleware explanation.The example could be more educational by showing how the middleware affects the request flow.
Consider enhancing the example to demonstrate middleware execution order:
func main() { app := fiber.New() - api := app.Group("/api", handler) // /api + // Middleware to log group access + logMiddleware := func(c fiber.Ctx) error { + fmt.Printf("Accessing group: %s\n", c.Route().Path) + return c.Next() + } + + api := app.Group("/api", logMiddleware) // /api - v1 := api.Group("/v1", handler) // /api/v1 + v1 := api.Group("/v1", logMiddleware) // /api/v1 v1.Get("/list", handler) // /api/v1/list v1.Get("/user", handler) // /api/v1/user
157-196: Add response expectations to Route examples.The examples would be more helpful with comments showing expected responses for each endpoint.
Consider adding response expectations:
app.Route("/test").Get(func(c fiber.Ctx) error { return c.SendString("GET /test") -}) +}) // Response: "GET /test" app.Route("/events").All(func(c fiber.Ctx) error { // Runs for all HTTP verbs first // Think of it as route-specific middleware! + // Log request method + fmt.Printf("Method: %s\n", c.Method()) return c.Next() }).Get(func(c fiber.Ctx) error { return c.SendString("GET /events") -}) +}) // Response: "GET /events"
53-53: Fix trailing spaces in markdown.The following lines contain trailing spaces which should be removed for better markdown compatibility:
- Line 53
- Line 92
- Line 625
- Line 635
Remove trailing spaces from these lines to improve markdown formatting consistency.
Also applies to: 92-92, 625-625, 635-635
🧰 Tools
🪛 GitHub Check: markdownlint
[failure] 53-53: Trailing spaces
docs/api/app.md:53:1 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 4] https://github.com/DavidAnson/markdownlint/blob/v0.35.0/doc/md009.md🪛 Markdownlint
53-53: Expected: 0 or 2; Actual: 4
Trailing spaces(MD009, no-trailing-spaces)
docs/api/bind.md (4)
67-80: Consider enhancing curl examples with expected responses.While the curl commands are clear and well-formatted, adding example responses would help users understand what to expect.
Add expected response examples after each curl command, for example:
```bash curl -X POST -H "Content-Type: application/json" --data "{\"name\":\"john\",\"pass\":\"doe\"}" localhost:3000 + +# Expected Response: +# { +# "name": "john", +# "pass": "doe" +# }Also applies to: 113-116, 149-152, 185-188, 221-224, 256-259, 293-296, 333-336, 374-377, 555-561 --- Line range hint `409-452`: **Consider adding error handling best practices for custom binders.** The custom binder example is well-structured, but it would be helpful to include guidance on error handling patterns. Add a note about error handling, for example: ```diff func (cb *customBinder) Parse(c fiber.Ctx, out any) error { // parse YAML body return yaml.Unmarshal(c.Body(), out) } + +// Note: When implementing custom binders, consider the following error handling best practices: +// 1. Return specific error types that can be handled appropriately +// 2. Include context in error messages +// 3. Handle both parsing and validation errors
Line range hint
485-561: Consider adding thread safety note for SetParserDecoder.The SetParserDecoder configuration affects global state and should be handled carefully in concurrent applications.
Add a warning note about thread safety:
// Add custom type to the Decoder settings fiber.SetParserDecoder(fiber.ParserConfig{ IgnoreUnknownKeys: true, ParserType: []fiber.ParserType{customTime}, ZeroEmpty: true, }) + +// Note: SetParserDecoder modifies global parser configuration. +// It should be called during application initialization before +// handling any requests to ensure thread safety.
Line range hint
566-604: Consider expanding validation rule examples.While the basic validation setup is well-documented, adding more common validation rule examples would be helpful.
Add more validation examples:
type Person struct { Name string `json:"name" validate:"required"` Age int `json:"age" validate:"gte=18,lte=60"` + Email string `json:"email" validate:"required,email"` + Phone string `json:"phone" validate:"required,e164"` + Role string `json:"role" validate:"required,oneof=admin user guest"` }.github/README.md (3)
531-550: WebSocket example needs error handling improvement.The WebSocket example could benefit from more robust error handling and connection management.
Consider adding:
- Connection upgrade error handling
- Proper connection close handling
- Message size limits
app.Get("/ws", websocket.New(func(c *websocket.Conn) { + // Set read/write deadlines and message size limits + c.SetReadDeadline(time.Now().Add(60 * time.Second)) + c.SetWriteDeadline(time.Now().Add(10 * time.Second)) defer c.Close() for { mt, msg, err := c.ReadMessage() if err != nil { - log.Println("read:", err) + if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) { + log.Printf("websocket error: %v", err) + } break }
Line range hint
727-739: Development section is well-structured but could use more detail.The development section lists available make commands but could benefit from more context about the development workflow.
Consider adding:
- Prerequisites for running make commands
- Expected output examples
- Troubleshooting common issues
706-710: External middleware table needs version compatibility information.The external middleware table should indicate version compatibility more clearly.
Consider adding a "Compatible Versions" column to help users choose the right middleware version:
| Middleware | Description | Compatible Versions | |------------|-------------|-------------------| | contrib | Third-party middlewares | v2.x, v3.x | | storage | Premade storage drivers... | v2.x, v3.x | | template | This package contains... | v3.x only |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
.github/README.md(24 hunks)docs/api/app.md(11 hunks)docs/api/bind.md(19 hunks)docs/api/hooks.md(7 hunks)docs/api/log.md(7 hunks)docs/api/redirect.md(10 hunks)
🧰 Additional context used
🪛 GitHub Check: markdownlint
docs/api/app.md
[failure] 53-53: Trailing spaces
docs/api/app.md:53:1 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 4] https://github.com/DavidAnson/markdownlint/blob/v0.35.0/doc/md009.md
[failure] 92-92: Trailing spaces
docs/api/app.md:92:1 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 4] https://github.com/DavidAnson/markdownlint/blob/v0.35.0/doc/md009.md
[failure] 625-625: Trailing spaces
docs/api/app.md:625:1 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 4] https://github.com/DavidAnson/markdownlint/blob/v0.35.0/doc/md009.md
[failure] 635-635: Trailing spaces
docs/api/app.md:635:1 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 4] https://github.com/DavidAnson/markdownlint/blob/v0.35.0/doc/md009.md
🪛 Markdownlint
docs/api/app.md
53-53: Expected: 0 or 2; Actual: 4
Trailing spaces
(MD009, no-trailing-spaces)
92-92: Expected: 0 or 2; Actual: 4
Trailing spaces
(MD009, no-trailing-spaces)
625-625: Expected: 0 or 2; Actual: 4
Trailing spaces
(MD009, no-trailing-spaces)
635-635: Expected: 0 or 2; Actual: 4
Trailing spaces
(MD009, no-trailing-spaces)
🪛 LanguageTool
docs/api/hooks.md
[uncategorized] ~10-~10: A comma is probably missing here.
Context: ...rt TabItem from '@theme/TabItem'; With Fiber you can execute custom user functions a...
(MISSING_COMMA_AFTER_INTRODUCTORY_PHRASE)
docs/api/redirect.md
[style] ~128-~128: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...e for the redirect. :::info It is used in conjunction with To, Route, and...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[style] ~147-~147: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...n for the redirect. :::info It is used in conjunction with the Route method. ::: ``...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
🔇 Additional comments (11)
docs/api/hooks.md (3)
Line range hint 24-32: LGTM! Clear and accurate type definitions.
The handler type definitions are well-documented with improved clarity in the comment.
135-163: LGTM! Well-structured example with proper imports and error handling.
The OnListen example has been improved with:
- Proper import organization
- Clear scheme determination logic
- Appropriate error handling for child processes
206-226: LGTM! Clear example with good separation of concerns.
The OnMount example demonstrates good practices:
- Separation of the handler function for reusability
- Clear demonstration of mount path access
- Proper error handling structure
docs/api/log.md (3)
12-12: LGTM! Title case formatting is correct.
The heading now follows proper title case convention.
26-28: LGTM! Clear and concise interface description.
The heading follows title case convention, and the interface description is well-written.
Line range hint 127-137: LGTM! Clear concurrent safety warning and usage explanation.
The documentation clearly warns about concurrent safety and explains the purpose of log levels.
docs/api/bind.md (3)
9-14: LGTM! Clear and accurate introduction.
The grammatical improvement and the clear warning about value lifetime enhance the documentation quality.
35-41: LGTM! Well-structured content type mapping.
The table clearly shows the relationship between content types and struct tags, with proper formatting.
Line range hint 611-644: LGTM! Comprehensive default fields documentation.
The supported types are clearly listed, and the examples effectively demonstrate the usage of default values.
.github/README.md (2)
Line range hint 42-53: Installation instructions are clear and well-structured.
The installation section provides clear prerequisites, setup steps, and helpful links to additional resources. The Go modules initialization instructions are particularly helpful for newcomers.
136-144: Good addition of package declarations in code examples.
The addition of complete package declarations and import statements in the code examples makes them more copy-paste friendly and helps prevent confusion for new users.
✅ Actions performedReviews paused. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Ready for review :-) |
gaby
changed the title
👍 Done |
Description
o1-minifrom OpenAIREADMEUSINGo1-minifrom OpenAIRelated to #2712
Changes introduced
Type of change