-
Notifications
You must be signed in to change notification settings - Fork 143
chore: add namespaced id or key #3677
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
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes This is a straightforward addition of a simple data model with basic validation logic. The implementation is clean and follows standard Go patterns for error handling. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
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 (1)
pkg/models/idorkey.go (1)
15-26: Consider aligning with the existing generic validation error helperThe validation logic itself is solid: both fields are checked and aggregated with
errors.Join, and you getnilwhen everything’s OK.If you want error handling to feel consistent with
Validate[T]inpkg/models/validator.go(which wraps inNewNillableGenericValidationError), you could have this method return the same wrapped error type instead of a plain joined error, e.g.:- return errors.Join(errs...) + return NewNillableGenericValidationError(errors.Join(errs...))That way callers don’t have to special‑case this type vs. anything validated through the generic helper.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/models/idorkey.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
pkg/models/idorkey.go
🧬 Code graph analysis (1)
pkg/models/idorkey.go (3)
app/common/namespace.go (1)
Namespace(13-15)pkg/ref/ref.go (1)
IDOrKey(9-12)pkg/models/validator.go (1)
Validate(16-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Artifacts / Container image
- GitHub Check: Migration Checks
- GitHub Check: Code Generators
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Lint
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
pkg/models/idorkey.go (1)
8-13: Clear, focused modeling of the “ID or key” caseThis type and its doc comment nicely capture the “we don’t yet know if it’s an ID or key” scenario, and the JSON shape (
namespace,idOrKey) is straightforward for API consumers. Looks good to me as-is.
Overview
Useful at places where we don't know yet if something is an ID or a key.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.