-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rebranding code in libvalkey #18
Conversation
In the PR for the cluster code Viktor had a fair comment about renaming files multiple times. Maybe we should move the renamed files to src/ directly here too? |
To |
To |
Co-authored-by: Björn Svensson <[email protected]>
Co-authored-by: Björn Svensson <[email protected]>
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.
LGTM
Reviewed in a scrolling speed of 1 page/second. 😎
Let's merge this and fix stuff afterwards? I believe that allows more people to work in parallel.
Some things to consider in the future follow-ups:
- Move fuzzing to
/tests/
- Move examples to
/examples/
or are they actually tests? - What include files to we like the users to see?
<valkey/valkey.h>
sync API, e.g. valkeyCommand()<valkey/async.h>
async API, e.g. valkeyAsyncCommand()<valkey/cluster.h>
Cluster sync API, e.g. valkeyClusterCommand()<valkey/cluster-async.h>
e.g. valkeyClusterAsyncCommand()? (or is it -AsyncCluster-?, i forgot)
- Move exported API include files to /include/? (Doesn't affect where gets installed.)
- Adapters .h files to /include/adapters/?
- Rename SSL to TLS?
- Or is it better to change as little as possible, so users can predictably just change "redis" to "valkey" and then everything else will work out of the box?
Sounds good. Feel free to merge. |
I left
Redis
in-tact in things like theCHANGELOG.md
and all the copyright headers but think I got everything else.Part of #11