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

Issue 798 - Auditing PFMERGE command docs #975

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

Kannav02
Copy link
Contributor

@Kannav02 Kannav02 commented Oct 6, 2024

This PR aims to find and fix inconsistencies in the documentation for the PFMERGE command as mentioned in the issue #798

The objectives of this PR can be simplified in the points below

  • Run the command and its examples in a local instance of DiceDB and make sure they work as per requirements
  • Compare the command with Redis's counterpart and make sure that the results are the same
  • In the official documentation, format the headings with the correct title as specified in the issue
  • Remove the Conclusion heading if it exists
  • Appropriate use of headers
  • Convert Parameters and Return Value to a markdown table format to ensure consistency

end goal - documentation should match the format followed by SET and be consistent with how the code works

Fixes #798

@AshwinKul28
Copy link
Contributor

AshwinKul28 commented Oct 7, 2024

HI @Kannav02 Thanks a lot for the changes, can you please add an error example (Invalid Usage) as well if possible?
Also please update the PR title.

@AshwinKul28 AshwinKul28 marked this pull request as ready for review October 7, 2024 12:24
@AshwinKul28 AshwinKul28 self-requested a review October 7, 2024 12:26
@Kannav02
Copy link
Contributor Author

Kannav02 commented Oct 7, 2024

sure i will update the PR title, also just to follow up ,
you've told in the issue to use Examples as the heading to display the examples for the CLI usage
but in the SET documentation, Example Usage is being utilised
should I still go ahead and make the change to Examples

thank you!

@Kannav02 Kannav02 changed the title Issue 798 Issue 798 - Auditing PFMERGE command docs Oct 7, 2024
@Kannav02 Kannav02 changed the title Issue 798 - Auditing PFMERGE command docs Issue 798 - Auditing PFMERGE command docs Oct 7, 2024
@Kannav02
Copy link
Contributor Author

Kannav02 commented Oct 9, 2024

@AshwinKul28 ,I think the PR should be good for review now

Thank you!

Copy link
Contributor

@lucifercr07 lucifercr07 left a comment

Choose a reason for hiding this comment

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

LGTM

@lucifercr07 lucifercr07 merged commit 290d8a6 into DiceDB:master Oct 11, 2024
1 check passed
sashpawar11 pushed a commit to sashpawar11/dice that referenced this pull request Oct 13, 2024
kakdeykaushik pushed a commit to kakdeykaushik/dice that referenced this pull request Oct 19, 2024
sa-k-shore pushed a commit to sa-k-shore/dice that referenced this pull request Oct 20, 2024
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.

Audit and make documentation for command PFMERGE consistent
3 participants