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

groups.json uses dash, command JSON uses underscore #178

Open
stockholmux opened this issue Oct 8, 2024 · 6 comments
Open

groups.json uses dash, command JSON uses underscore #178

stockholmux opened this issue Oct 8, 2024 · 6 comments

Comments

@stockholmux
Copy link
Member

I'm working on valkey-io/valkey-io.github.io#150 using the groups.json file to get human readable descriptions from command JSON group fields.

Everything corresponds between the files except sorted set in command JSON files are grouped as sorted_set and the groups.json uses sorted-set.

I can work around this but it's annoying that we're not consistent. I'd be in favour of changing groups.json to reflect the field in the command JSON. Will this break the man page generation script?

@zuiderkwast
Copy link
Contributor

I'd be in favour of changing groups.json to reflect the field in the command JSON. Will this break the man page generation script?

No, I would just change the man page stuff at the same time.

Will it break the website?

@stockholmux
Copy link
Member Author

@zuiderkwast Nope. I would have a bit of dead code to transform the underscores to dashes but it's fine.

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Dec 9, 2024

I was just about to change it to underscore when I found another place where sorted-set is used with a dash:

127.0.0.1:6379> COMMAND DOCS zrank
1# "zrank" => 
   1# "summary" => "Returns the index of a member in a sorted set ordered by ascending scores."
   2# "since" => "2.0.0"
   3# "group" => "sorted-set"
   4# "complexity" => "O(log(N))"
   5# "history" => 
      1~ 1) "7.2.0"
         2) "Added the optional `WITHSCORE` argument."
   6# "arguments" => 
      1) 1# "name" => "key"
         2# "type" => "key"
         3# "display_text" => "key"
         4# "key_spec_index" => (integer) 0
      2) 1# "name" => "member"
         2# "type" => "string"
         3# "display_text" => "member"
      3) 1# "name" => "withscore"
         2# "type" => "pure-token"
         3# "display_text" => "withscore"
         4# "token" => "WITHSCORE"
         5# "flags" => 1~ optional

And one line in the reply schema describing the reply above: https://github.com/valkey-io/valkey/blob/8.0.1/src/commands/command-docs.json#L84

Suddenly I'm not certain we should change this anymore. Changing the reply of COMMAND DOCS can be considered a breaking change.

@stockholmux
Copy link
Member Author

stockholmux commented Dec 9, 2024

I'll twitch every time I see the line that transforms the - to the _ but it's not worth a breaking change.

What a maddeningly inconsistent thing. Oh well, feel free to close.

@stockholmux
Copy link
Member Author

Although we should probably document that changing this file valkey-doc could make a breaking change in valkey-server 😮

@zuiderkwast
Copy link
Contributor

I'll twitch every time I see the line that transforms the - to the _ but it's not worth a breaking change.

I'll show you the code that turns sorted-set into sorted_set for COMMAND DOCS. Beware, it may get you into an infinite-twitch-loop, so please hold someone's hand. It's C code generated by Python code. An array of names that must be in the same order as another array with other names in another file in a different language, namely server.h...

Although we should probably document that changing this file valkey-doc could make a breaking change in valkey-server 😮

No, it doesn't. The code doesn't depend on the doc repo, so we can change the doc repo without considering it a breaking change. That won't solve the inconsistency though, as long as the name sorted-set exists in the output of COMMAND DOCS, which might be (arguably?) the recommended way to get this metadata rather than reading the command JSON files directly. I just mean that if we want to solve the inconsistency completely, including in COMMAND DOCS, that would be a breaking change.

Anyway, I don't really think it's worth changing sorted_sets to sorted-sets in groups.json at this point. We could have used COMMAND DOCS instead of the command JSON files and then we would have hade sorted-sets everywhere.

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

No branches or pull requests

2 participants