-
-
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
Revert "Update README.md (#3165)" #3166
Conversation
This reverts commit 9dd3d94.
WalkthroughThe pull request involves a modification to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/README.md (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.github/README.md (2)
Line range hint
1-24
: Consider updating Go version requirementThe README states that Fiber requires Go version 1.22 or higher. However, Go 1.22 is not yet released (as of October 2024).
Let's verify the Go version requirement:
#!/bin/bash # Check for Go version requirement in go.mod grep "go 1." go.modPlease update the Go version requirement to reflect the actual minimum supported version.
Line range hint
1-24
: Update version information in READMEThe README still mentions Fiber v3 as being in beta. However, the PR objectives mention that this is a revert of a previous change. It's unclear if v3 is still in beta or if it has been released.
Let's verify the current version status:
Please update the version information in the README to accurately reflect the current status of Fiber v3.
@@ -73,7 +73,7 @@ func main() { | |||
app := fiber.New() | |||
|
|||
// Define a route for the GET method on the root path '/' | |||
app.Get("/", func(c *fiber.Ctx) error { | |||
app.Get("/", func(c fiber.Ctx) error { |
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.
Update function signature in example code
The function signature in the example code has been updated from func(c *fiber.Ctx) error
to func(c fiber.Ctx) error
. This change removes the pointer to the fiber.Ctx
type.
This change may have significant implications:
- It could potentially break existing code that relies on the pointer receiver.
- It might impact performance, as passing the context by value instead of by pointer could lead to unnecessary copying.
- It's inconsistent with the Fiber v2.x API, which uses pointer receivers for the context.
I recommend reverting this change or providing a clear explanation for why this change was made. If this is intentional, consider adding a note about the API change and its implications for users upgrading from previous versions.
- app.Get("/", func(c fiber.Ctx) error {
+ app.Get("/", func(c *fiber.Ctx) error {
// Send a string response to the client
return c.SendString("Hello, World 👋!")
})
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
app.Get("/", func(c fiber.Ctx) error { | |
app.Get("/", func(c *fiber.Ctx) error { |
This reverts commit 9dd3d94.