dice icon indicating copy to clipboard operation
dice copied to clipboard

Audit and make documentation for command `SETBIT` consistent

Open arpitbbhayani opened this issue 1 year ago • 7 comments

The documentation of the command [SETBIT](https://dicedb.io/commands/setbit/) 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

  • to understand all possible return values
  • to understand all possible error values and conditions
  • to understand different parameters the command accepts

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 see any "Conclusion" section, remove it
  • the headers (h1, h2, h3) should be appropriately used
  • the CLI prompt should be 127.0.0.1:7379> and not anything else
  • Use markdown tables for parameters and return values (ref SET command documentation)
  • wrap any command or paramter in backticks (`) to highlight them
  • the section header should be capitalize as used in the SET command documentation

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.

arpitbbhayani avatar Sep 29 '24 05:09 arpitbbhayani

Hey could you please assign this to me

Yashasv-Prajapati avatar Sep 30 '24 17:09 Yashasv-Prajapati

@Yashasv-Prajapati assigned, thanks for contributing.

lucifercr07 avatar Oct 01 '24 13:10 lucifercr07

Hello @Yashasv-Prajapati,

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 avatar Oct 09 '24 02:10 arpitbbhayani

Hey, apologies for the delay, I have this document prepared, and was trying to go through the codebase to understand in depth about any other issues that might occur with this command.

This is the attached doc for the changes and the current behaviour - https://docs.google.com/document/d/1ZSs3KbHE4fe90Wk6sHFjYXw1M27w2kd41Gk3QDF2VDQ/edit?usp=sharing

In the last section, I have highlighted 2 major potential changes that I think should be done as per the current code, please let me know if I should update these in the docs in SETBIT.md file.

Thanks and apologies again for the delay

Yashasv-Prajapati avatar Oct 09 '24 07:10 Yashasv-Prajapati

Hey @arpitbbhayani @lucifercr07 Any updates on this? I highlighted the 2 major issues that I found.

  1. In the command negative offset is not handled, which causes the server to crash in cases when a key does not exist and needs to be created or the offset is larger than the bit length of the key's corresponding value. So if I do setbit mykey -1 1, and mykey is not yet set the server will crash and close the connection.

  2. Only strings are considered valid for this command, if I do > set mykey 123 it stores it as an integer, and so the setbit does not apply on it, if I do setbit mykey 24 1, it throws the following error (error) WRONGTYPE Operation against a key holding the wrong kind of value.

Please let me know, If I should update the documentation with these warnings, or will it first be fixed and then only the documentation be updated?

Yashasv-Prajapati avatar Oct 15 '24 16:10 Yashasv-Prajapati

Hi @Yashasv-Prajapati , please go ahead and create the issue for these inconsistencies. If you would like to work on the issue, please comment or mention the same in the description. Thanks

apoorvyadav1111 avatar Oct 15 '24 17:10 apoorvyadav1111

Hello @Yashasv-Prajapati,

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 avatar Oct 24 '24 13:10 arpitbbhayani

closed as part of #1017

apoorvyadav1111 avatar Nov 14 '24 04:11 apoorvyadav1111