-
Notifications
You must be signed in to change notification settings - Fork 30
Add support for usernames #72
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
| /// - Parameters: | ||
| /// - connectionInitialDatabase: The optional database index to initially connect to. The default is `nil`. | ||
| /// Redis by default opens connections against index `0`, so only set this value if the desired default is not `0`. | ||
| /// Redis by default opens connections against index `0`, so only set this value if the desired default is not `0`. |
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.
Was this intentional?
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.
yes. nicer format.
Joannis
left a 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.
Check the breaking change. The rest looks good
| connectionUsername: String? = nil, | ||
| connectionPassword: String? = nil, | ||
| connectionDefaultLogger: Logger? = nil, | ||
| tcpClient: ClientBootstrap? = nil |
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.
I think this will break, as we now have two instances of .init() where all arguments are defaulted to nil
| } | ||
| } | ||
|
|
||
| private func send0( |
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.
❤️
| /// - `defaultLogger`: The optional prototype logger to use as the default logger instance when generating logs from the connection. | ||
| /// If one is not provided, one will be generated. See `RedisLogging.baseConnectionLogger`. | ||
| /// - Throws: `RedisConnection.Configuration.ValidationError` if invalid arguments are provided. | ||
| public init( |
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.
Actually, isn't this the same scenario? Also two cases where a public init(address: ...) would be ambiguous?
No description provided.