valkey-io / valkey-doc

Content for website and man pages
Other
21 stars 31 forks source link

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

Closed stockholmux closed 3 months ago

stockholmux commented 4 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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. 😉

stockholmux commented 3 months ago

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 commented 3 months ago

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.