scylla-operator icon indicating copy to clipboard operation
scylla-operator copied to clipboard

Scylla Operator documentation menus are polluted with sublevel items

Open tnozicka opened this issue 1 year ago • 9 comments

Menu in our docs are polluted with subitems that are incorrectly defined, like the Results in the main menu coming from /helm page. Screenshot from 2024-02-23 13-05-32

These items have incorrectly defined level which is likely why they started appearing in the main menu, after some changes in the framework.

What's most bothering is that the build doesn't fail and they silently break the docs.

/cc @rzetelskik @dgarcia360

Shortlist of offenders

$ find ./docs/source/ -name '*.md' -exec grep -HnE -e '^# ' {} \; | grep -v -e ':1:#'
./docs/source/support/must-gather.md:60:# Create a copy of the existing kubeconfig and
./docs/source/support/must-gather.md:61:# replace user authentication using yq, or by adjusting the fields manually.
./docs/source/contributing.md:31:# Create the scylla operator repo path
./docs/source/contributing.md:34:# Navigate to the local repo path and clone your fork
./docs/source/contributing.md:37:# Clone your fork, where <user> is your GitHub account name
./docs/source/contributing.md:45:# Add 'upstream' to the list of remotes
./docs/source/contributing.md:48:# Verify the remote was added
./docs/source/contributing.md:68:# Ensure all your remotes are up to date with the latest
./docs/source/contributing.md:71:# Create a new branch that is based off upstream master.  Give it a simple, but descriptive name.
./docs/source/contributing.md:72:# Generally it will be two to three words separated by dashes and without numbers.
./docs/source/contributing.md:109:# Inspect your commit history to determine if you need to squash commits
./docs/source/contributing.md:112:# Rebase the commits and edit, squash, or even reorder them as you determine will keep the history clean.
./docs/source/contributing.md:113:# In this example, the last 5 commits will be opened in the git rebase tool.
./docs/source/eks.md:14:# Edit according to your preference
./docs/source/eks.md:18:# From inside the examples/eks folder
./docs/source/generic.md:320:# Run a benchmark with 10 jobs, with 6 cpus and 50.000.000 operations each.
./docs/source/generic.md:321:# Each Job will throttle throughput to 30.000 ops/sec for a total of 300.000 ops/sec.
./docs/source/gke.md:13:# Edit according to your preference
./docs/source/gke.md:18:# From inside the examples/gke folder
./docs/source/gke.md:22:# Example:
./docs/source/gke.md:23:# ./gke.sh -u [email protected] -p gke-demo-226716 -z us-west1-b
./docs/source/helm.md:243:# Results
./docs/source/helm.md:314:# Monitoring
./docs/source/helm.md:332:# Cleanup

tnozicka avatar Feb 23 '24 12:02 tnozicka

Probably introduced in https://github.com/scylladb/scylla-operator/pull/1540, not the changes to the framework. I sent the PR to suppress the warnings, but the headings are still being used unreasonably in some of the source files. It doesn't seem to be a bug though, just an incorrect use of headings?

What's most bothering is that the build doesn't fail and they silently break the docs.

What do you mean, does it fail to build?

Shortlist of offenders

Most of those are comments, which are not rendered as menu items. Look like the only offending file is helm.md?

rzetelskik avatar Feb 23 '24 12:02 rzetelskik

What's most bothering is that the build doesn't fail and they silently break the docs.

What do you mean, does it fail to build?

it builds but our menu is broken (has extra fields at the top level that shouldn't be there)

tnozicka avatar Feb 23 '24 12:02 tnozicka

Probably introduced in https://github.com/scylladb/scylla-operator/pull/1540

that PR is removing the suppression and I see no suppression config in master, so this should warn and warnings become errors, so it should fail, but it doesn't fail...

tnozicka avatar Feb 23 '24 12:02 tnozicka

There's no reason for it to fail though - the headings are increasing consecutively. Looks like I screwed up a couple of them in that PR, I'll send a fix for the headings.

rzetelskik avatar Feb 23 '24 12:02 rzetelskik

I don't think there's anything else we could do about it though? Seems like generating a menu item for every top-level heading is the expected behaviour. We could maybe have something to prevent more than one top-level heading per file, but I can imagine this coming back to bite us later on.

rzetelskik avatar Feb 23 '24 12:02 rzetelskik

it may be a different one but there should be only ONE top level header (title) per page

tnozicka avatar Feb 23 '24 12:02 tnozicka

it may be a different one but there should be only ONE top level header (title) per page

@dgarcia360 would you be able to assist here? What would be the right way to achieve that?

Maybe we could have some dedicated syntax for the page title / menu item, instead of rendering each top-level heading as a menu item? Or, at the least, we could add a custom warning for source files with more than one top-level headings?

rzetelskik avatar Feb 23 '24 13:02 rzetelskik

/assign @dgarcia360

tnozicka avatar Mar 11 '24 08:03 tnozicka

@tnozicka: GitHub didn't allow me to assign the following users: dgarcia360.

Note that only scylladb members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to this:

/assign @dgarcia360

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tnozicka @rzetelskik I apologize for missing this issue. If I'm slow to respond, please feel free to contact @annastuchlik or send me an email.

I'm reviewing this and will share more details after doing some tests.

Question: Are all page headers currently fixed? What do you want is to get an error if there is more than one H1 on a page?

dgarcia360 avatar Apr 16 '24 17:04 dgarcia360

Question: Are all page headers currently fixed?

Yes, we merged https://github.com/scylladb/scylla-operator/pull/1750 to mitigate this.

What do you want is to get an error if there is more than one H1 on a page?

That'd be ideal.

rzetelskik avatar Apr 24 '24 19:04 rzetelskik