DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.7k stars 1.06k forks source link

Audit and make documentation for command `SREM` consistent #809

Open arpitbbhayani opened 1 month ago

arpitbbhayani commented 1 month ago

The documentation of the command [SREM](https://dicedb.io/commands/srem/) present in the docs could have become stale. We need your help to ensure that the documentation is complete, correct, and consistent.  Completing this issue will give you much-needed exposure to how the command is implemented and the different parameters it accepts. Hence, this is a great way to build a good understanding of the project and its functionality. While still making a significant contribution. Here's what you need to do:

  1. Go through the documentation of the command and run all the examples, making sure they all work as expected
  2. If the same command is present in Redis, then the expected output of the DiceDB command is the same as the Redis output
  3. If the command is not in Redis, then as per your judgment, raise an issue with this template or fix the documentation and raise a pull request.
  4. The documentation should contain the following sections in the following order
    • short and concise introduction paragraph about the command covering what it does
    • Syntax
    • Parameters: List all the parameters the command accepts, and ignore the section if there are no parameters
    • Return values: List all possible return values and under what condition
    • Behaviour: Describe the behavior of the command, what it does, how it does it, some internal specifics if any
    • Errors: List all possible errors the command can throw and under what condition
    • Examples: List all possible examples of the command and the expected output. Assume CLI implementation

Note: The title of the sections should be the exact strings mentioned above, like "Examples", "Return values", etc. Note: The description of the command in Frontmatter of the command.md file is the first paragraph (introduction) of the command.

Go through the DiceDB/dice repository and explore the command implementation

Please use the documentation of the SET command as the reference point and structure the documentation of this command in a very similar way, including the following points

If you find any inconsistencies, please fix the documentation and raise the PR. The core idea of this exercise is to ensure that the documentation is consistent, correct, and complete.

Make sure you comment on the other issues you created (if any) as a comment on this issue and also any PR (if any) that you created. Thank you for picking this up and contributing to the DiceDB. It means a ton.

danielwetan commented 1 month ago

hi @arpitbbhayani can I take up this issue? this seems a good starting point for me, thank you

lucifercr07 commented 1 month ago

@danielwetan assigned, thanks for contributing.

arpitbbhayani commented 1 month ago

Hello @danielwetan,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

arpitbbhayani commented 2 weeks ago

Hello @danielwetan,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

shubzp commented 6 days ago

Hello @arpitbbhayani @lucifercr07

Please assign this to me.

lucifercr07 commented 6 days ago

@shubzp assigned, thanks for contributing.

shubzp commented 5 days ago

I run all the commands in the document. Everything is working as expected. The only issue was the output in the CLI. Redis shows the type (integer) and that is what is stated in our document as well. However, we do not show the type. Ideally the printReply() in the CLI should have covered this. I don't think there is a todo in the db repo. I will raise an issue in the CLI project.

REDIS 127.0.0.1:6379> SADD k1 v1 v2 v3 v4 (integer) 2 127.0.0.1:6379> SREM k1 v2 (integer) 1 127.0.0.1:6379> SADD k2 "v1" "v2" "v3" (integer) 3 127.0.0.1:6379> SREM k2 "v2" (integer) 1 127.0.0.1:6379> SMEMBERS k1 1) "v1" 2) "v3" 3) "v4" 127.0.0.1:6379> SMEMBERS k2 1) "v1" 2) "v3"

DICE dicedb> SADD k1 v1 v2 v3 v4 4 dicedb> SREM k1 v2 1 dicedb> SADD k2 "v1" "v2" "v3" 3 dicedb> SREM k2 "v2" 1 dicedb> SMEMBERS k1 1) v4 2) v1 3) v3 dicedb> SMEMBERS k2 1) v3 2) v1

Till it is fixed, I would raise a PR to fix SREM document.

vanshavenger commented 5 days ago

@shubzp try it with redis-cli -p 7379 once

shubzp commented 5 days ago

@vanshavenger You want me to try querying dicedb with redis-cli?

shubzp commented 5 days ago

@arpitbbhayani @lucifercr07 I created this issue in dicedb-cli