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

#816 Docs : Audited Docs for Copy command #1140

Merged

Conversation

onlybond
Copy link
Contributor

This PR closes Issue : #816

Have changed the docs to be consistent and used SET Command docs for reference

Do lemme know if there are any changes to be done

@onlybond onlybond changed the title Audited Copy command for docs consistency Docs : Audited Docs for Copy command Oct 18, 2024
@apoorvyadav1111 apoorvyadav1111 self-assigned this Oct 18, 2024
@apoorvyadav1111 apoorvyadav1111 changed the title Docs : Audited Docs for Copy command #816 Docs : Audited Docs for Copy command Oct 18, 2024
@apoorvyadav1111 apoorvyadav1111 added the documentation Improvements or additions to documentation label Oct 18, 2024
Copy link
Contributor

@apoorvyadav1111 apoorvyadav1111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @onlybond , Thanks for the changes. DiceDB currently does not support databases hence the argument destination db is not being used by the server. A user can pass the parameter, It will be ignored. Therefore, please update any such reference where it is mentioned that the destination key will be updated in the destination DB.
Thanks

@onlybond
Copy link
Contributor Author

Hey @apoorvyadav1111 , Thanks for reviewing the PR, As you said DiceDB doesn't support databases, so in the docs should i remove all the references which are related to db? So in the parameters only 3 parameters will be given as of now?

@apoorvyadav1111
Copy link
Contributor

So you can remove references which mention that the key will be inserted in database, You can mention in the description about this. But please take that as an argument as we do take it as an argument, but just ignore it. It act as sort of placeholder.

Copy link
Contributor

@apoorvyadav1111 apoorvyadav1111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, we have created referenced for documentation for easy reference and consistency which can found here: docs/command_docs_template.md and docs/sample_command_docs.md

@apoorvyadav1111
Copy link
Contributor

Hi @onlybond, Hope you are well. I am eager to know more about your progress on this issue. Please reach out to me if you need any clarifications.

@onlybond
Copy link
Contributor Author

hello @apoorvyadav1111 I have mentioned about the the key to be stored in another database in the notes. until diceDB supports databases its viable to mention it in the notes section.

Copy link
Contributor

@apoorvyadav1111 apoorvyadav1111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Thanks for making the changes. Please align the Error sections to the template.
Thanks,
Apoorv

@onlybond
Copy link
Contributor Author

hi there I have made the changes. please have a review on it.

Copy link
Contributor

@apoorvyadav1111 apoorvyadav1111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left one minor comment. I think we are good to go after this change

Comment on lines 44 to 49
1. `WRONGTYPE Operation against a key holding the wrong kind of value`:
- This error occurs if the source key holds a value that is not compatible with the `COPY` operation.
2. `ERR no such key`:
- This error occurs if the source key does not exist.
3. `ERR target key name is busy`:
- This error occurs if the destination key already exists and the `REPLACE` option is not specified.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. `WRONGTYPE Operation against a key holding the wrong kind of value`:
- This error occurs if the source key holds a value that is not compatible with the `COPY` operation.
2. `ERR no such key`:
- This error occurs if the source key does not exist.
3. `ERR target key name is busy`:
- This error occurs if the destination key already exists and the `REPLACE` option is not specified.
1. `Wrong type of value or key`:
- Error message `(error) WRONGTYPE Operation against a key holding the wrong kind of value`:
- This error occurs if the source key holds a value that is not compatible with the `COPY` operation.
2. `Not existent key`:
- Error message: `(error) ERR no such key`
- This error occurs if the source key does not exist.

@onlybond
Copy link
Contributor Author

All changes done. LGTM

@apoorvyadav1111 apoorvyadav1111 merged commit f3a69b8 into DiceDB:master Oct 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants