docs: note removal of context keys#3707
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes outdated documentation references to the ContextKey configuration option from middleware documentation and consolidates the explanation in the changelog. The changes clean up documentation by removing redundant ContextKey mentions from individual middleware sections and adding a single comprehensive note explaining that all middlewares have dropped this option.
- Removes specific
ContextKeyremoval notes from CSRF middleware documentation - Adds a general note in the migration section explaining the removal across all middlewares
- Consolidates documentation to avoid repetition and improve clarity
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@ReneWerner87 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughDocumentation update to whats_new.md describing middleware API shifts: context-based data storage with FromContext helpers, removal of ContextKey and KeyLookup patterns, CSRF/Session renames and extractor adoption, BasicAuth and KeyAuth API adjustments, timeout.New signature change, and updates to testing config. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Fiber as Fiber App
participant MW as Middleware
participant Ctx as Request Context
participant Handler as Route Handler
Client->>Fiber: HTTP Request
Fiber->>MW: Invoke middleware chain
rect rgba(200,230,255,0.3)
note right of MW: Middleware extracts data (e.g., token, session, request ID)
MW->>Ctx: Store unexported values
end
MW-->>Fiber: Next()
Fiber->>Handler: Call handler
rect rgba(220,255,220,0.3)
Handler->>Ctx: requestid.FromContext / csrf.TokenFromContext / session.FromContext / keyauth.TokenFromContext
Ctx-->>Handler: Return value(s)
end
Handler-->>Fiber: Response
Fiber-->>Client: HTTP Response
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @ReneWerner87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request updates the project documentation to reflect changes in middleware configurations, specifically the removal of the ContextKey option across various middlewares. It aims to ensure the documentation accurately reflects the current API, guiding users on how to retrieve data from context using new helper functions instead of user-defined keys.
Highlights
- Documentation Cleanup: Removed specific mentions of
ContextKeyremoval from the CSRF middleware documentation, streamlining thewhats_new.mdfile. - General ContextKey Removal Note: Added a new, general changelog entry in
whats_new.mdto inform users that all middlewares have dropped theirContextKeyconfiguration option, emphasizing the use of dedicated helper functions for data retrieval.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request refactors the documentation to centralize information about the removal of the ContextKey option in middlewares. The changes correctly remove specific notes for CSRF and add a general note in the migration guide. This improves the clarity and conciseness of the documentation. I've suggested a small wording improvement to make the new general note even more direct and less redundant.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/whats_new.md (2)
2003-2004: Tighten scope and add guidance for third-party middleware“All middlewares” may be interpreted as including community/contrib packages that haven’t migrated. Suggest narrowing the scope to Fiber-maintained middleware and adding a brief note for external packages.
Apply this diff:
-All middlewares have also dropped their `ContextKey` configuration option. Values are no longer stored under user-defined keys; instead, retrieve them with the helper functions described below. +All Fiber-maintained middlewares have dropped their `ContextKey` configuration option. Values are no longer stored under user-defined keys; retrieve them via the helper functions listed below. For contrib or third-party middleware, consult their documentation to confirm whether `ContextKey` is still supported or which helper to use.
2003-2004: Optional: add one-line migration nudge to make next step explicitA short “do this now” sentence helps readers translate the note into action.
Proposed follow-up sentence (append after the paragraph):
Migration: remove any `ContextKey` fields from your middleware configs and replace `c.Locals("<key>")` reads with the corresponding `Xxx.FromContext(c)` helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/whats_new.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/whats_new.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
📚 Learning: 2024-11-08T04:10:42.990Z
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
Applied to files:
docs/whats_new.md
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
docs/whats_new.md
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Applied to files:
docs/whats_new.md
🔇 Additional comments (2)
docs/whats_new.md (2)
2003-2004: LGTM: clear migration callout for ContextKey removalThis succinctly communicates the cross-middleware breaking change and fits well in the migration section.
2003-2004: ContextKey References Cleaned UpA repo-wide search for remaining
ContextKeymentions confirms that only the intended reference in the migration note remains.• docs/whats_new.md:2003 – the single, expected mention in the “What’s New” guide
• No other occurrences in docs/ or middleware/**/*.mdAll other
ContextKeyusages have been removed.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Summary
Testing
make audit(fails: EncodeMsg passes lock by value; session/data_msgp.go lock copy)make generatemake betteralign(fails: package requires newer Go version go1.25 (application built with go1.24))make modernize(fails: package requires newer Go version go1.25 (application built with go1.24))make formatmake test(fails: Test_App_BodyLimit_LargerThanDefault timeout)https://chatgpt.com/codex/tasks/task_e_68a9f436ea1883268b1f69302fb4ca60