Conversation
Found via `codespell -L te,nd` and `typos --hidden --format brief`
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughGrammar corrections in panic messages within binder/msgpack.go and aligned test expectations in binder/msgpack_test.go. A test function in client/request_test.go was renamed for spelling consistency. No logic, control flow, or API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 @kianmeng, 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 focuses on improving the overall code quality and consistency by addressing various typos identified through automated tools like codespell and typos. The changes span across different file types, including configuration files, Go source code, and test files, ensuring greater accuracy and professionalism in the codebase.
Highlights
- Configuration File Update: Corrected a typo from "bumb" to "bump" in the .github/labeler.yml file, affecting the regex for dependency labels.
- Go Source Code and Test File Correction: Fixed the word "explicits" to "explicit" in panic messages within binder/msgpack.go and its corresponding test file binder/msgpack_test.go.
- Test Function Renaming: Renamed the Test_Request_Unsupport_Protocol function to Test_Request_Unsupported_Protocol in client/request_test.go to correct a grammatical error.
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 correctly fixes several typos across the codebase. My review includes suggestions to further improve the grammar of one of the updated error messages for better clarity, along with the necessary changes in the corresponding tests.
|
|
||
| func UnimplementedMsgpackMarshal(_ any) ([]byte, error) { | ||
| panic("Must explicits setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") | ||
| panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") |
There was a problem hiding this comment.
While fixing the typo from explicits to explicit is good, the grammar can be further improved for clarity. "Must explicit setup" is slightly awkward. Using an adverb explicitly and splitting setup into set up would make the error message more natural and professional.
| panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") | |
| panic("Must explicitly set up Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") |
|
|
||
| func UnimplementedMsgpackUnmarshal(_ []byte, _ any) error { | ||
| panic("Must explicits setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") | ||
| panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") |
There was a problem hiding this comment.
Similar to the comment above, let's improve the grammar of this panic message for better clarity. "Must explicit setup" should be changed to "Must explicitly set up".
| panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") | |
| panic("Must explicitly set up Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") |
| defer func() { | ||
| if r := recover(); r != nil { | ||
| require.Contains(t, r, "Must explicits setup Msgpack") | ||
| require.Contains(t, r, "Must explicit setup Msgpack") |
There was a problem hiding this comment.
To align with the suggested change in binder/msgpack.go, this test assertion should be updated to check for the grammatically improved error message.
| require.Contains(t, r, "Must explicit setup Msgpack") | |
| require.Contains(t, r, "Must explicitly set up Msgpack") |
| defer func() { | ||
| if r := recover(); r != nil { | ||
| require.Contains(t, r, "Must explicits setup Msgpack") | ||
| require.Contains(t, r, "Must explicit setup Msgpack") |
There was a problem hiding this comment.
To align with the suggested change in binder/msgpack.go, this test assertion should also be updated to check for the grammatically improved error message.
| require.Contains(t, r, "Must explicit setup Msgpack") | |
| require.Contains(t, r, "Must explicitly set up Msgpack") |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
binder/msgpack.go (1)
28-29: Tighten grammar in panic messages (“explicitly set up”)Minor nit: “Must explicit setup” is still slightly awkward. Consider “You must explicitly set up …” for correct phrasing. If you adopt this, update the corresponding tests too.
Apply this diff here:
- panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") + panic("You must explicitly set up Msgpack. See docs: https://docs.gofiber.io/next/guide/advance-format#msgpack")and here:
- panic("Must explicit setup Msgpack, please check docs: https://docs.gofiber.io/next/guide/advance-format#msgpack") + panic("You must explicitly set up Msgpack. See docs: https://docs.gofiber.io/next/guide/advance-format#msgpack")Also applies to: 32-33
binder/msgpack_test.go (1)
113-114: If adjusting the panic text, align the test substring accordinglyOnly needed if you take the suggested grammar tweak in binder/msgpack.go. Update the expected substring to match.
- require.Contains(t, r, "Must explicit setup Msgpack") + require.Contains(t, r, "You must explicitly set up Msgpack")and
- require.Contains(t, r, "Must explicit setup Msgpack") + require.Contains(t, r, "You must explicitly set up Msgpack")Also applies to: 126-127
📜 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 ignored due to path filters (1)
.github/labeler.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
binder/msgpack.go(1 hunks)binder/msgpack_test.go(2 hunks)client/request_test.go(1 hunks)
🔇 Additional comments (1)
client/request_test.go (1)
820-827: Test name grammar fix looks goodRenaming to Test_Request_Unsupported_Protocol improves clarity and aligns with common terminology. No functional changes; assertions remain valid.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3695 +/- ##
=======================================
Coverage 91.81% 91.81%
=======================================
Files 114 114
Lines 11498 11498
=======================================
Hits 10557 10557
Misses 681 681
Partials 260 260
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:
|
|
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
|
🥳 🥳 🥳 🥳 🥳 |
Description
Fix typos found via
codespell -L te,ndandtypos --hidden --format briefFixes # (issue)
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md