valkey icon indicating copy to clipboard operation
valkey copied to clipboard

Support connection schemes: valkey://, valkeys://.

Open zhulipeng opened this issue 1 year ago • 2 comments

This patch try to:

  1. Support valkey:// and valkeys:// scheme, retain the original Redis scheme for compatibility.
  2. Add unit tests for valid URI.

Fixes: https://github.com/valkey-io/valkey/issues/198 Fixes: https://github.com/valkey-io/valkey/issues/200

zhulipeng avatar Apr 04 '24 10:04 zhulipeng

AH!! I didn't review the tests. If there is more coming, better mark the PR as draft. :D

I'll review the tests now.

zuiderkwast avatar Apr 04 '24 12:04 zuiderkwast

Please also update the top comment to describe the changes (don't just link issue). For example, mention we supports valkey://, and also retains the original redis schema for compatibility, and then we adds some tests to cover these cases.

also if you have a PR directly, there is actually no need to create a new issue (there is no discussion or tracking in the issue so it is actually worthless)

enjoy-binbin avatar Apr 04 '24 13:04 enjoy-binbin

I'm going to merge this. It's the least surprising schemes for any forked clients and for users moving from redis-cli to valkey-cli.

We can change back if there are protests.

zuiderkwast avatar Apr 23 '24 01:04 zuiderkwast