docs: clarify handler parameters and execution order in routing methods#3512
docs: clarify handler parameters and execution order in routing methods#3512sixcolors wants to merge 2 commits into
Conversation
WalkthroughThe changes update documentation comments and examples for HTTP method registration functions and related documentation files. They clarify the execution order of route handlers, specify the main handler as a required parameter, and illustrate the updated method signatures, including the use of a slice for multiple HTTP methods in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FiberApp
participant Middleware1
participant Middleware2
participant MainHandler
Client->>FiberApp: HTTP Request
FiberApp->>Middleware1: Execute (if provided)
Middleware1->>Middleware2: Execute (if provided)
Middleware2->>MainHandler: Execute (main handler, always last)
MainHandler-->>Client: Response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the documentation for routing methods by explicitly defining the required main handler and optional additional handlers, and by clarifying their execution order.
- Updated the method signatures in documentation to clearly separate the main handler from additional handlers.
- Revised code examples and comments in multiple files to reflect the new ordering and type safety improvements.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/whats_new.md | Updated changelog and examples to clarify the new routing method signature and order. |
| docs/partials/routing/handler.md | Updated handler documentation to indicate the explicit separation of handlers. |
| app.go | Revised inline documentation for routing methods to explain the new handler execution order. |
Comments suppressed due to low confidence (1)
app.go:788
- The documentation comments for the routing methods are repetitive. Consider referencing a shared documentation block or guideline to reduce redundancy and ease future maintenance.
func (app *App) Get(path string, handler Handler, handlers ...Handler) Router {
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3512 +/- ##
==========================================
- Coverage 83.88% 83.84% -0.05%
==========================================
Files 120 120
Lines 12278 12278
==========================================
- Hits 10299 10294 -5
- Misses 1555 1559 +4
- Partials 424 425 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/partials/routing/handler.md (1)
29-34: Explicit “Handler Execution Order” section is helpful but refine punctuation.Consider removing the colon after "handler" or making list-item punctuation consistent (either omit trailing punctuation on both items or include periods).
🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...r from additional handlers: -handler: The main route handler (required, execu...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app.go(1 hunks)docs/partials/routing/handler.md(2 hunks)docs/whats_new.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/partials/routing/handler.md
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...r from additional handlers: - handler: The main route handler (required, execu...
(UNLIKELY_OPENING_PUNCTUATION)
docs/whats_new.md
[style] ~1471-~1471: Try moving the adverb to make the sentence clearer.
Context: ...P method registration methods have been updated to explicitly require a handler, followed by optional additional handle...
(SPLIT_INFINITIVE)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (17)
docs/partials/routing/handler.md (4)
11-23: Clarify required main handler and optional additional handlers in signatures.The updated comments and method signatures clearly enforce that a main handler is mandatory and additional handlers are optional, improving API clarity and type safety.
37-40: Approve simple handlers example.The example demonstrates usage without additional handlers and aligns with the new signature.
46-53: Approve additional-handlers example.The annotated execution order (middleware → main handler) is clear and correctly positioned.
56-61: Approve multiple-methods example.Demonstrates the new
Addsignature with a slice of methods and correct handler order.app.go (11)
791-800: Document Get handler parameters and execution order.
806-815: Document Head handler parameters and execution order.
821-830: Document Post handler parameters and execution order.
836-845: Document Put handler parameters and execution order.
850-859: Document Delete handler parameters and execution order.
865-874: Document Connect handler parameters and execution order.
880-889: Document Options handler parameters and execution order.
895-903: Document Trace handler parameters and execution order.
910-919: Document Patch handler parameters and execution order.
924-933: Document Add method parameters and execution order.
940-949: Document All method parameters and execution order.docs/whats_new.md (2)
241-246: Clarify v2 vs v3 handler signature enforcement.The migration guide now explicitly states that the main handler is required and additional handlers follow, improving reader understanding.
253-275: Update method signatures in migration diff.The diff accurately reflects signature changes for all HTTP registration methods and
Add, aligning with v3 API.
| // Get registers a route for GET methods that requests a representation | ||
| // of the specified resource. Requests using GET should only retrieve data. | ||
| // | ||
| // The first parameter 'handler' is the main route handler (executed last). |
There was a problem hiding this comment.
this is wrong, the first handler is executed first
@sixcolors
app.Get("/users", mainHandler, handler1, handler2)
// Execution order: mainHandler -> handler1 -> handler2
in the order in which they were specified
ReneWerner87
left a comment
There was a problem hiding this comment.
unfortunately wrong description
|
The handler execution order was changed in Fiber v3 (in current beta as of Dec 2024 release) but reverted to the original behavior in PR #3321, which is in main but not yet released in a v3 beta. @ReneWerner87 to avoid confusion for users who adapted to the beta's execution order, I recommend highlighting this change in the next v3 release notes and adding a note to the documentation to clarify the switch back, as it may impact code written for the v3 beta release. |
|
ok good point |
Enhance documentation to explicitly define the required main handler and optional additional handlers in routing methods. This change improves clarity and type safety by clearly outlining the execution order of handlers.