valkey-io / valkey

A flexible distributed key-value datastore that supports both caching and beyond caching workloads.
https://valkey.io
Other
17.05k stars 636 forks source link

Rename internal struct and function names (redisCommand, redisDb, etc.) #144

Closed zuiderkwast closed 7 months ago

zuiderkwast commented 7 months ago

Internal struct and function names. Replace "redis" => "server". This isn't really a trademark problem. We just want to make the code a little more Redis agnostic.

Note 1: We shouldn't change deps/hiredis/ (redisContext, redisCommand(), redisBufferWrite(), etc.) since it's a vendored dependency. If we want to change it, it should probably be by forking it and vendoring the forked dependency instead.

Note 2: Don't change the module API here. See #146.

9bany commented 7 months ago

@zuiderkwast Can you assign this issue to me please ?

zuiderkwast commented 7 months ago

@0del Yes, thanks for your work. :)

If there are big changes, consider making multiple small PRs, like one for each struct name or so.

9bany commented 7 months ago

Thx @zuiderkwast

9bany commented 7 months ago

redisBufferWrite (function) -> Did you mean this ? @zuiderkwast https://github.com/valkey-io/valkey/blob/1629e28f86c598bf8663c1a0ede81b68bc04a8c2/deps/hiredis/hiredis.h#L330

zuiderkwast commented 7 months ago

Right, it should not be renamed. I'll remove it from the list.

zuiderkwast commented 7 months ago

@0del Our first release will be based on the 7.2 branch. Since this internal code isn't really a trademark problem and cherry-picking these changes to that branch will cause a lot of merge conflicts, we decided to release this when we branch off unstable in our next major release.

That said, let's finish this. We're fast.

9bany commented 7 months ago

well done @zuiderkwast

zuiderkwast commented 7 months ago

Well done @0del!

zuiderkwast commented 7 months ago

@0del Did we forget something?

I can find a few things using git grep 'redis[A-Z]' now (under src/):

redisBitpos redisCommandTable (Also in utils/generate-command-code.py very important) redisNodeFlags redisNodeFlagsTable redisLrand48 redisSrand48 redisLrand48 redisSrand48 redisProtocolToLuaType_* (Let's not touch them now. It is about the RESP protocol.) redisAsciiArt redisOutOfMemoryHandler redisProcTitleGetVariable redisSupervisedUpstart redisSupervisedSystemd redisTestProc redisTest redisTests redisDebug redisDebugMark

9bany commented 7 months ago

Note 1 -> i will handle in this issue https://github.com/valkey-io/valkey/issues/192

zuiderkwast commented 7 months ago

@0del if you're looking for something to do, do you want to fix the functions I posted in a comment above? (All in one PR should be fine, if you search/replace these exact names.)

zuiderkwast commented 7 months ago

Ah, I see you're on it. Great! :smile:

zuiderkwast commented 7 months ago

So next is redisBitpos, ..., redisDebugMark?

9bany commented 7 months ago

redisBitpos redisCommandTable (Also in utils/generate-command-code.py very important) redisNodeFlags redisNodeFlagsTable redisLrand48 redisSrand48 redisLrand48 redisSrand48 redisProtocolToLuaType_* (Let's not touch them now. It is about the RESP protocol.) redisAsciiArt redisOutOfMemoryHandler redisProcTitleGetVariable redisSupervisedUpstart redisSupervisedSystemd redisTestProc redisTest redisTests redisDebug redisDebugMark

i fixed in this PR: https://github.com/valkey-io/valkey/pull/191 @zuiderkwast

9bany commented 7 months ago

i still looking for something to do 😄 @zuiderkwast

zuiderkwast commented 7 months ago

All remaining occurrences of redisXxxx are from hiredis now? I'll consider this issue done.

9bany commented 7 months ago

redisXxxx are from hiredis

I will handle this issue @zuiderkwast

ah, please check https://github.com/valkey-io/valkey/pull/203