Add support for usernames by fabianfett · Pull Request #72 · swift-server/RediStack

@fabianfett

Joannis

/// - 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`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intentional?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. nicer format.

Joannis

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Joannis

/// - `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(

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?

Joannis