-
Notifications
You must be signed in to change notification settings - Fork 45
chore: align workflows with Go 1.25 #115
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
Conversation
Summary of ChangesHello @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 primarily focuses on upgrading the project's core dependencies to align with the latest versions of Go (1.25) and the Fiber web framework (v3). This update ensures compatibility with newer language features and framework improvements, while also updating the module's own versioning to Highlights
Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
|
Warning Rate limit exceeded@ReneWerner87 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 31 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)
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. WalkthroughMigrated module to Fiber v3 and updated documentation for Go 1.25+. Reworked Swagger static serving to use Fiber v3 static middleware, changed handler signatures (pointer → value), adjusted redirect and config handling, and updated tests to parse Content-Type with mime.ParseMediaType. Minor .gitignore comment added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App as Fiber v3 App
participant Static as Static Middleware
participant Handler as Swagger Handler
participant Docs as Swagger UI/Docs
Client->>App: GET /swagger/* or /swagger
App->>Static: route to static middleware
Static->>Static: attempt to locate static asset
alt Asset found
Static-->>Client: 200 Serve asset (js/css/index.html)
else Asset not found
Static->>Handler: forward request
Handler->>Handler: compute basePrefix (sync.Once)
Handler->>Handler: get forwarded prefix (value param)
Handler->>Handler: prepare cfgCopy and doc path
Handler-->>Client: Redirect().Status(...).To(...) or render index using cfgCopy
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential areas needing extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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. Comment |
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.
Code Review
This pull request updates the project to be compatible with Go 1.25 and Fiber v3. The changes include updating the Go version directive, module dependencies, and adapting the code to the new APIs in Fiber v3. The documentation in README.md has also been updated to reflect these changes. The changes are well-executed and necessary for the upgrade. I have one minor suggestion regarding code formatting in the README.md file for consistency.
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: 1
🧹 Nitpick comments (3)
swagger_test.go (1)
108-114: Better Content-Type assertion. Also close response bodies.Parsing media type is correct. Add resp.Body.Close() to avoid descriptor leaks in long-running test suites.
Apply near the response creation:
- resp, err := app.Test(req) + resp, err := app.Test(req) if err != nil { t.Fatal(err) } + t.Cleanup(func() { if resp != nil && resp.Body != nil { _ = resp.Body.Close() } })README.md (2)
11-11: Go 1.25 requirement noted. Add CI/toolchain hint for reliability.Consider mentioning GOTOOLCHAIN=auto (Go 1.21+) so developers and CI auto-fetch 1.25 even if the runner image lags.
Example:
- export GOTOOLCHAIN=auto
- Or use actions/setup-go with go-version-file and check-latest: true
19-21: Command block clarity.Since Go 1.17+, prefer go install for tools; keep go get solely for adding library deps.
Clarify:
- go install github.com/swaggo/swag/cmd/swag@latest (tool)
- go get github.com/gofiber/swagger/v2 (library)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
.github/workflows/gotidy.ymlis excluded by!**/*.yml.github/workflows/test.ymlis excluded by!**/*.ymlgo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (4)
.gitignore(1 hunks)README.md(3 hunks)swagger.go(4 hunks)swagger_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Learnt from: grivera64
PR: gofiber/fiber#3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:15:26.529Z
Learning: In Fiber v3, net/http handlers (http.Handler, http.HandlerFunc, or raw func(http.ResponseWriter, *http.Request)) can be passed directly to routing methods like app.Get(), app.Post(), etc. The framework automatically detects and wraps them internally via toFiberHandler/collectHandlers. The github.com/gofiber/fiber/v3/middleware/adaptor package is legacy and should not be suggested for tests or code using native net/http handler support.
📚 Learning: 2024-11-15T07:56:21.623Z
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Applied to files:
swagger.go
📚 Learning: 2025-10-16T07:15:26.529Z
Learnt from: grivera64
PR: gofiber/fiber#3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:15:26.529Z
Learning: In Fiber v3, net/http handlers (http.Handler, http.HandlerFunc, or raw func(http.ResponseWriter, *http.Request)) can be passed directly to routing methods like app.Get(), app.Post(), etc. The framework automatically detects and wraps them internally via toFiberHandler/collectHandlers. The github.com/gofiber/fiber/v3/middleware/adaptor package is legacy and should not be suggested for tests or code using native net/http handler support.
Applied to files:
swagger.go
🔇 Additional comments (9)
.gitignore (1)
15-15: Comment-only addition looks good.Helpful clarification above vendor/. No functional impact.
swagger_test.go (2)
4-4: Robust header parsing dependency added.Adding mime is appropriate for Content-Type normalization in tests.
9-9: No changes required—app.Test() calls conform to Fiber v3 signature.Both invocations at lines 97 and 148 correctly pass only the required
*http.Requestparameter. The optionalconfig ...TestConfigparameter is not needed here and correctly omitted.README.md (3)
28-28: Module import path is correct.Library major version v2 path aligns with current releases; Fiber is v3. No conflict.
33-34: Import snippet aligns with the module path.Matches the get command; consistent for users copy-pasting.
42-44: Example targets Fiber v3 correctly.Imports look right (swagger v2 + fiber v3).
swagger.go (3)
10-13: Imports updated for Fiber v3 and static FS.Correct packages for v3 (fiber, static, utils v2).
70-70: Redirect API usage looks good.Fluent Redirect().Status(...).To(...) matches v3 style.
77-99: Original code correctly assumesGetReqHeaders()returnsmap[string][]string; review comment's core concern is unfounded.GetReqHeaders() returns map[string][]string, confirming the original implementation correctly iterates header values as a slice. The proposed simplification using
c.Get()with CSV splitting would change behavior (single concatenated header vs. multiple header instances per HTTP spec), not fix an API misunderstanding.Likely an incorrect or invalid review comment.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f2405aa3788326b7d39afa93ceca81
Summary by CodeRabbit
Documentation
Chores
Tests