-
Notifications
You must be signed in to change notification settings - Fork 1
Implement support for renaming collections #53
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
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.
Pull Request Overview
This PR implements support for renaming collections by adding a new rename operation to the collections handler and CLI command. The changes also improve the validation error messaging for better clarity.
Key Changes:
- Added
RenameCollectionhandler method with ULID validation and error handling - Added
collections renameCLI command with proper argument validation - Enhanced validation error message formatting and clarity
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/validation/error.go | Updated validation error formatting and improved collection name requirement message |
| cmd/bore-cli/app/handler/collections.go | Implemented RenameCollection handler with ULID parsing and validation |
| cmd/bore-cli/app/commands.go | Added rename subcommand to collections command |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if collectionID == "" { | ||
| return cli.Exit("collection id is required", 1) | ||
| } else if newName == "" { | ||
| return cli.Exit("new name is required", 1) | ||
| } |
Copilot
AI
Oct 21, 2025
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.
These empty string checks are redundant since lines 102-106 already validate that exactly 2 arguments are provided via c.NArg(). The c.Args().Get() calls will never return empty strings when the argument count validation passes.
| if collectionID == "" { | |
| return cli.Exit("collection id is required", 1) | |
| } else if newName == "" { | |
| return cli.Exit("new name is required", 1) | |
| } | |
| // Redundant empty string checks removed; argument count validation above suffices. |
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.
Wrap both checks with strings.TrimSpace instead?
0105427 to
7ac4a76
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
7ac4a76 to
1bff89b
Compare
RenameCollectionon collections handlercollections renamecommandErrormethod on validation error type