Skip to content

Conversation

@aosasona
Copy link
Owner

  • Implement RenameCollection on collections handler
  • Add collections rename command
  • Update Error method on validation error type

@aosasona aosasona linked an issue Oct 20, 2025 that may be closed by this pull request
@aosasona aosasona requested a review from Copilot October 21, 2025 04:44
Copy link
Contributor

Copilot AI left a 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 RenameCollection handler method with ULID validation and error handling
  • Added collections rename CLI 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.

Comment on lines +111 to +115
if collectionID == "" {
return cli.Exit("collection id is required", 1)
} else if newName == "" {
return cli.Exit("new name is required", 1)
}
Copy link

Copilot AI Oct 21, 2025

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

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?

@aosasona aosasona force-pushed the 49-implement-collection-renaming branch from 0105427 to 7ac4a76 Compare October 21, 2025 19:17
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@aosasona aosasona force-pushed the 49-implement-collection-renaming branch from 7ac4a76 to 1bff89b Compare October 21, 2025 19:23
@aosasona aosasona merged commit 4f6dad1 into master Oct 21, 2025
7 checks passed
@aosasona aosasona deleted the 49-implement-collection-renaming branch October 21, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement collection renaming subcommand

2 participants