-
Notifications
You must be signed in to change notification settings - Fork 141
Fix import policy api on windows #1328
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
base: master
Are you sure you want to change the base?
Fix import policy api on windows #1328
Conversation
…olicy-api-on-windows-460
WalkthroughPath handling logic in importAPIPolicy.go is updated to normalize Windows-style backslashes to forward slashes before deriving the policy name, ensuring consistent cross-platform behavior when splitting directory paths. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
import-export-cli/impl/importAPIPolicy.go (2)
79-81: Consider usingfilepath.Base()for cleaner cross-platform path handling.The current fix correctly normalizes Windows backslashes, but Go's standard library provides
filepath.Base()which extracts the final path element in a platform-agnostic way.Apply this diff for a more concise and idiomatic solution:
- tmpPath2 := strings.ReplaceAll(tmpPath, "\\", "/") - policyPaths := strings.Split(tmpPath2, "/") - policyName := policyPaths[len(policyPaths)-1] + policyName := filepath.Base(tmpPath)
84-84: Consider usingfilepath.Join()for path construction consistency.While not changed in this PR, this line uses string concatenation with a hardcoded forward slash for path construction. For consistency with other parts of the codebase (e.g., line 195) and proper cross-platform support, consider using
filepath.Join().- originalFilePath := tmpPath + "/" + file.Name() + originalFilePath := filepath.Join(tmpPath, file.Name())
Summary by CodeRabbit
Release Notes