Skip to content

Adds valkey-json module commands documentation to the website. #243

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

roshkhatri
Copy link
Member

@roshkhatri roshkhatri commented Mar 8, 2025

One of the three PRs Required for module documentation to be included on the website

Related PRs:
valkey-io/valkey-json#42

Will wait for valkey-io/valkey-io.github.io#212 to het merged so we can use the framework to display documentation

We have merged the bloom changes in PR too as it depends on that, we would only review the changes related to JSON for this PR

@hpatro
Copy link
Contributor

hpatro commented Mar 31, 2025

@roshkhatri is it ready to be reviewed ?

@roshkhatri
Copy link
Member Author

Not yet

Copy link

@joehu21 joehu21 left a comment

Choose a reason for hiding this comment

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

josn.mset.md is missing.

@roshkhatri roshkhatri marked this pull request as ready for review April 1, 2025 08:27
@roshkhatri roshkhatri requested review from joehu21, hpatro and zuiderkwast and removed request for joehu21 April 1, 2025 08:29
JSON.ARRAPPEND <key> <path> <json> [json ...]
```

* key - required, Redis key of document type
Copy link
Member

Choose a reason for hiding this comment

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

Not a redis key

Copy link
Contributor

Choose a reason for hiding this comment

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

JSON should be added in this file as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

JSON resp3 reply needs to be added.

"JSON.ARRAPPEND": [
"* If the path is enhanced syntax:",
" * [Array reply](../topics/protocol.md#arrays): Array of integers representing the new length of the array at each path.",
" * [Null reply](../topics/protocol.md#nulls): For each path where the value is not an array.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Null reply is available in resp3, for resp2 it would be nil reply.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any ValkeyModule_ReplyWithMap usage in JSON module so presume all the response are in array format. So, this looks good to me.

Copy link
Contributor

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

High level looks good. This PR needs to be rebased once the Bloom PR is merged.

@KarthikSubbarao
Copy link
Member

KarthikSubbarao commented Apr 2, 2025

This PR can be rebased now. The bloom changes are merged in.

Also, the spellchecker needs to get addressed

https://github.com/valkey-io/valkey-doc/blob/502c0995e2afdccdcd2008fbebc5b77401953d99/wordlist

You can add words here ^ to ignore them

zackcam and others added 11 commits April 2, 2025 23:31
…to generate bloom man pages

Signed-off-by: zackcam <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
…creation and spelling

Signed-off-by: zackcam <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: zackcam <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Making changes based on review comments

Co-authored-by: KarthikSubbarao <[email protected]>
Signed-off-by: zackcam <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: zackcam <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Roshan Khatri <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Roshan Khatri <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Nikhil-Manglore and others added 8 commits April 2, 2025 23:31
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Roshan Khatri <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
Signed-off-by: Nikhil Manglore <[email protected]>
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.

7 participants