sc-redis
sc-redis copied to clipboard
Consider options rename
Problem
I think that the option names for Redis host and port should be renamed to be more Redis specific. When looking at the configuration in your SocketCluster project, it is unclear what the host
and port
properties are used for.
Proposal
Old:
const socketCluster = new SocketCluster({
brokerOptions: {
host: '54.204.147.15',
port: 6379
}
});
New:
const socketCluster = new SocketCluster({
brokerOptions: {
redis: {
host: '54.204.147.15',
port: 6379
// Also supports easily passing additional options in the future
}
}
});
@jondubois I would be happy to work on this change if we decide this is the right direction.
It looks a bit redundant, but I totally agree that's how it should be. + it's possible to implement with no breaking changes.
@MegaGM It doesn't really matter that it's possible to make this a non-breaking change. Since this module is pre-1.0.0 any new published version may contain breaking changes since npm will lock it down anyway. In my opinion, we should not support both and drop support for the old way.
@austinkelleher Sounds great. Feel free to go ahead. I think it's OK as a breaking change if we bump the major version to 1.0.0. So long as the snippets in this repo's README are also updated :)