Conversation
appleboy
commented
Jan 13, 2026
appleboy
commented
Collaborator
- Replace legacy initialization functions with a unified NewStore() using the Option Pattern
- Add extensive configuration options for connection, authentication, Redis, and session settings
- Implement strict validation for configuration and connection options at initialization
- Remove code duplication and improve separation of concerns in store setup
- Update documentation and usage examples for the new API
- Add a comprehensive migration guide for upgrading from v1 to v2
- Update module path to github.com/boj/redistore/v2 for semantic versioning
- Improve error messages and fail-fast behavior for invalid configurations
- Add a new test suite with thorough coverage for option validation and error handling
- Deprecate v1 API and document migration steps
- Enhance developer experience with clearer, more flexible, and self-documenting options
- Replace legacy initialization functions with a unified NewStore() using the Option Pattern - Add extensive configuration options for connection, authentication, Redis, and session settings - Implement strict validation for configuration and connection options at initialization - Remove code duplication and improve separation of concerns in store setup - Update documentation and usage examples for the new API - Add a comprehensive migration guide for upgrading from v1 to v2 - Update module path to github.com/boj/redistore/v2 for semantic versioning - Improve error messages and fail-fast behavior for invalid configurations - Add a new test suite with thorough coverage for option validation and error handling - Deprecate v1 API and document migration steps - Enhance developer experience with clearer, more flexible, and self-documenting options Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
- Refactor error construction to use multiline return statements for clarity - Replace if-else chains with a switch statement for connection option selection - Update tests to build expected error strings in multiple lines for consistency with implementation Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a major v2 refactoring that replaces four legacy initialization functions with a unified NewStore() function using the Option Pattern. The change aims to improve API usability, reduce code duplication, and provide more flexible configuration.
Changes:
- Unified API with
NewStore()replacingNewRediStore(),NewRediStoreWithDB(),NewRediStoreWithPool(), andNewRediStoreWithURL() - Added 15+ configuration options for connection, authentication, Redis, and session settings
- Implemented comprehensive validation for configuration at initialization time
- Added extensive test suite (
redistore_options_test.go) with 23+ tests for option validation - Updated module path to
github.com/boj/redistore/v2for semantic versioning - Added
MIGRATION.mdguide andCHANGELOG.mddocumentation
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
redistore.go |
Core implementation: added Option Pattern with storeConfig, validation, and unified NewStore() function; removed legacy initialization functions |
redistore_test.go |
Updated all test cases to use new NewStore() API with options |
redistore_options_test.go |
New comprehensive test suite for option validation and error handling |
go.mod |
Updated module path to v2 (github.com/boj/redistore/v2) |
README.md |
Complete rewrite with new API examples, configuration reference, and usage patterns |
MIGRATION.md |
New migration guide with step-by-step instructions and common pitfalls |
CHANGELOG.md |
New changelog documenting v2.0.0 breaking changes and improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+464
to
+468
| cfg.address.address, | ||
| cfg.username, | ||
| cfg.password, | ||
| cfg.db, | ||
| ) |
There was a problem hiding this comment.
When using WithURL, the authentication credentials (username, password) and database selection (db) specified via WithAuth/WithPassword/WithDB options are ignored. The URL should contain all connection parameters. This inconsistent behavior could confuse users who try to mix WithURL with other configuration options. Consider either documenting this limitation clearly or validating that these options are not used together.
Comment on lines
+490
to
+491
| return pool, nil | ||
| } |
There was a problem hiding this comment.
Documentation states "keyPairs - One or more key pairs for cookie encryption and authentication" which suggests support for multiple key pairs, but the function signature only accepts a single []byte instead of variadic ...[]byte. This is inconsistent with the documentation and the actual usage pattern for key rotation.
Suggested change
| return pool, nil | |
| } | |
| // keyPairs - Key pair bytes for cookie encryption and authentication. | |
| // Typically generated with securecookie.GenerateRandomKey. |
- Change NewStore to accept key pairs as [][]byte instead of []byte, enabling proper support for key rotation - Add Keys and KeysFromStrings helper functions for easier key pair creation - Update documentation and migration guides to reflect the new key pairs format and usage - Add detailed examples and explanations for key rotation and helper usage - Update all code and tests to use the new key pairs format and helpers - Add tests for key rotation and the new helper functions Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Refactor repeated Redis connection error strings into constants for improved maintainability - Update tests to use the new error constants instead of hardcoded error messages Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.