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

/topics/cluster-spec.md has outdated info #140

Closed
stockholmux opened this issue May 31, 2024 · 8 comments · Fixed by #153
Closed

/topics/cluster-spec.md has outdated info #140

stockholmux opened this issue May 31, 2024 · 8 comments · Fixed by #153

Comments

@stockholmux
Copy link
Member

In pre-publishing review (#91), I found the following issues:

What is described in this document is implemented in Redis OSS 3.0 or greater.

Drop this reference to an ancient version.

CLUSTER MEET ip port

Use of code backticks for inline code.

Copyright 2010 Salvatore Sanfilippo (adapted to Valkey coding style)

This should probably revert back to 'redis' since Salvatore didn't adapt it to Valkey.

All code blocks should move to backticks/syntax highlighter for readability.

Finally, looks like there are 196 references that need to be updated to 'primary'.

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Jun 19, 2024

I'm removing the reference to Redis OSS 3 and reverting to "adapted to Redis coding style".

All code blocks should move to backticks/syntax highlighter for readability.

There's no usable language to use for syntax highlighting of Valkey commands like CLUSTER MEET ip port. If there would be a language we could mark it as, I'd buy the change to backticks, but without it, I'm going to ignore this comment. We're not changing just for the sake of changing.

@stockholmux
Copy link
Member Author

It's not a change for the sake of changing.

CLUSTER MEET ip port doesn't render properly as a code block in either GitHub nor on the website.

Screenshot 2024-06-20 at 1 47 18 PM Screenshot 2024-06-20 at 1 47 12 PM

@zuiderkwast
Copy link
Contributor

Do you mean it's indented less than 4 spaces? Because there's no difference between

CLUSTER MEET ip port

and

CLUSTER MEET ip port

@zuiderkwast
Copy link
Contributor

It's under a bullet list item so it'd need to be indented more. Or switch to backticks to avoid the problem of nested indentation.

The crap about syntax highlighing is what made me disregard the entire comment as trolling.

@stockholmux
Copy link
Member Author

I am (generally) not trolling. I actually think it should be fixed as in #149.

The use of the code block here is super inconsistent with the rest of the document.

@zuiderkwast
Copy link
Contributor

Hehe, I know! 😄 But syntax highlighting for valkey commands and other pseudo code is still hard to take serious, unless you have a specific language it can be tagged as. 😉

zuiderkwast pushed a commit that referenced this issue Jun 20, 2024
Fixes issue (#140) with `CLUSTER MEET ip port` rendering improperly.

Signed-off-by: Kyle J. Davis <[email protected]>
@stockholmux
Copy link
Member Author

stockholmux commented Jun 20, 2024

On the subject of fenced code blocks and languages...

Screenshot 2024-06-20 at 2 59 27 PM

Of the 383ms it takes to build the website, I'm fairly sure most of that time is emitting this warning about valkey-cli to stdout hundreds of times.

I don't care that much because the whole website builds faster than a browser takes to rendering a single page, but ideally we should get rid valkey-cli on code blocks (unless you know something I don't!)

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Jun 20, 2024

Yes I can tell you what I know. Thanks for bringing it up.

Previously this was some hugo code which isn't valid markdown, so it broke rendering (or rendered as visible braces, etc ) when fed to markdown tools. I mass-replaced them to keep this as a place-holder in case we ever want to use it for interactive examples or maybe add a custom syntax highlighter for these valkey-cli sessions. (I assume that could be done, at least in theory.)

This valkey-cli tag is just an unknown language, but it's still valid markdown and is ignored by any markdown tool and doesn't affect the rendering of the page (though a warning is printed in this cas), so it's unintrusive in that sense.

I'm fine with deleting it if you don't think we'll have any use for it.

zuiderkwast added a commit to zuiderkwast/valkey-doc that referenced this issue Jul 3, 2024
…alkey-io#141

* Remove junk line numbers in code examples and incorrect output in the valkey-cli prompt in examples in in cluster-tutorial.
* Remove reference to Redis OSS 3 and revert an occurrence of "adapterd to Redis coding style", since that's what this old code was.

Signed-off-by: Viktor Söderqvist <[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 a pull request may close this issue.

2 participants