fix: remove trailing carriage return in windows version check#265
fix: remove trailing carriage return in windows version check#265gaby merged 2 commits intogofiber:masterfrom
Conversation
Summary of ChangesHello @SamTV12345, 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 addresses a critical bug impacting the migration tool's functionality on Windows. The core problem stemmed from the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a crash that occurs on Windows when checking the version. The issue was caused by a carriage return being part of the version string, which is now properly handled by using strings.TrimSpace. The fix is effective. I've added a suggestion to include a new test case to cover this specific scenario and prevent future regressions. Overall, this is a good contribution.
WalkthroughThe file-reading function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/version_test.go`:
- Around line 101-105: Rename the test case from "can read version from windows
clrf file" to "can read version from windows crlf file" and fix the fixture in
the content variable by removing the stray '\r' that appears between the module
path and version (the sequence `"\tgithub.com/gofiber/fiber\r v0.0.0-..."`),
ensuring lines use proper CRLF pairs like `"\r\n"` only; update the t.Run name
and the content string in the test function so the fixture accurately represents
real Windows CRLF line endings.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/version_test.go
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Sorry AI. This is actually by design lol. |
|
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 |
|
@SamTV12345 Thank you, tagged. https://github.com/gofiber/cli/releases/tag/v0.11.3 |
|
You're welcome! |
Pull Request Guide
When I first checked out the migration tool I was wondering why such a tool breaks at first try. I then digged deeper and realized that Windows is the problem returning the version with a \r at the end which the semver parser declines. This is now fixed with a trimSpace. See the stacktrace below:
Please provide enough information so that others can review your pull request:
Explain the details for making this change. What existing problem does the pull request solve?
Commit formatting
Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me
Summary by CodeRabbit
Bug Fixes
Tests