Skip to content
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

Merged
merged 10 commits into from
Jun 20, 2024

Conversation

michael-grunder
Copy link
Collaborator

I left Redis in-tact in things like the CHANGELOG.md and all the copyright headers but think I got everything else.

Part of #11

@bjosv
Copy link
Collaborator

bjosv commented Jun 19, 2024

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?

@michael-grunder
Copy link
Collaborator Author

To libvalkey/src in this PR or to the root?

libvalkey/adapters/glib.h Outdated Show resolved Hide resolved
libvalkey/async.c Outdated Show resolved Hide resolved
libvalkey/test.sh Outdated Show resolved Hide resolved
@bjosv
Copy link
Collaborator

bjosv commented Jun 19, 2024

To libvalkey/src in this PR or to the root?

To <repo root>/src/, then we would have them in the right place from start. Then I'll move the cluster source there aswell so we have them mixed.

michael-grunder and others added 2 commits June 19, 2024 12:39
Co-authored-by: Björn Svensson <[email protected]>
Co-authored-by: Björn Svensson <[email protected]>
libvalkey/adapters/glib.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@zuiderkwast zuiderkwast left a 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?

@michael-grunder
Copy link
Collaborator Author

Sounds good. Feel free to merge.

@bjosv bjosv merged commit cd45a7c into valkey-io:main Jun 20, 2024
1 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants