Make the generated navigation and the repository's YAML yamllint-clean - #263
Open
seebi wants to merge 3 commits into
Open
Make the generated navigation and the repository's YAML yamllint-clean#263seebi wants to merge 3 commits into
seebi wants to merge 3 commits into
Conversation
nav.yml is generated by tools/build_navigation.py, and its output satisfied none of yamllint's default rules: no document start, and block sequences emitted at the same column as their parent key rather than indented under it. That is 280 indentation findings plus a missing `---` on a file nothing hand edits. Emit a `---`, and dump through a Dumper subclass whose increase_indent forces `indentless=False` so sequences nest beneath their key, as `indent-sequences` expects. The indent stays at two. Four is unreachable: `indentation` defaults to `spaces: consistent`, so every step in the file must be the same width, and a `- ` item prefix already costs two columns for the mapping key inside a sequence item. PyYAML's `indent=4` pads the dash to `- key:` and trips `hyphens`; re-indenting by hand to ` - key:` trips `indentation` instead. Both are recorded in a comment next to the dump so this does not get reopened. A handful of entries pair a long title with a long docs-relative path and exceed 120 columns. YAML folds plain scalars only at internal spaces, never after the `: `, so they cannot be wrapped - the generated header carries a `yamllint disable rule:line-length` directive rather than relaxing the rule for the whole repository. The regenerated nav.yml parses equal to its predecessor; the diff is entirely whitespace and the navigation is unchanged.
Preparation for the yamllint check added next: these are every finding it reports, fixed so the check can be turned on green. The .pages navigation sources make up the bulk. 64 gain a `---` document start and 30 a missing newline at end of file. One, deploy-and-configure/configuration/explore/dataplatform/.pages, is the only nested one and the only real error: its sub-list sat at column 8 where `spaces: consistent` wants 10, since the key it hangs off is at column 6 and the file's step is 4. Verified afterwards that the generated nav.yml is byte-identical, so none of this moves a page in the navigation. The three Helm values files under docs/ are downloadable examples rather than snippet-included, so nothing on a rendered page changes. They pick up an off-by-one nested indent, a doubled space after `secretName:`, bracket spacing, a missing final newline, a document start, and a comment reflowed under 120 columns. .github/ISSUE_TEMPLATE/bug.yml keeps its over-long line: the URL alone exceeds the limit and a folded scalar cannot help, because a line break inside one becomes a space and would break the Markdown link. It gets a `yamllint disable-line` directive instead, and the file still parses identically. mkdocs.yml and .gitpod.yml pick up comment spacing and a document start.
Nothing in this repository ran yamllint, so YAML style was unenforced and drifted - the previous two commits are the backlog that had accumulated. The config is committed rather than left to a user-level one. CI runs `task check`, and a GitHub runner has no ~/.config/yamllint/config, so without a repository config the stage would silently fall back to yamllint's stricter upstream defaults - line-length 80 above all - and fail on nearly every file while passing locally. .yamllint therefore restates the settings it is meant to mirror, and says so in a comment. `yaml-files` has to name `.pages` explicitly. yamllint selects by filename and its built-in globs only cover *.yaml, *.yml and .yamllint, so the 65 extensionless navigation sources - the files nav.yml is generated from - would otherwise go unchecked while the generated output was held to the rules. truthy is relaxed for keys so that `on:` in a GitHub workflow stops being read as the boolean. Wired into `task check` between check:rumdl and check:navigation, and into pre-commit with a filter that also fires on .pages. yamllint joins the dev dependency group; the lock file is refreshed because CI installs from it.
Test Results1 files ±0 2 suites ±0 0s ⏱️ ±0s For more details on these failures, see this check. Results for commit 1411bd2. ± Comparison against base commit 7b0e371. ♻️ This comment has been updated with latest results. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
nav.ymlis generated, and its output satisfied none of yamllint's defaultrules. Fixing that turned out to be worth doing properly, so this also brings
the rest of the repository's YAML in line and adds a check so it stays that way.
poetry run yamllint .now exits 0 across all 79 YAML files, up from 999findings on
nav.ymlalone.Three commits, in order
nav: generate yamllint-clean YAML— the generator emits a---and dumpsthrough a
Dumpersubclass that indents block sequences beneath their parentkey, which is what
indent-sequencesexpects. The regeneratednav.ymlparsesequal to its predecessor: the diff is entirely whitespace and no page moves.
yaml: fix style issues across the tracked YAML files— the backlog that hadaccumulated while nothing enforced style. Mostly the 64
.pagessources gaininga document start and a final newline. Verified afterwards that the generated
nav.ymlis byte-identical, so none of it touches the navigation.check: add a yamllint stage backed by a committed config—task check:yamllint, wired intotask checkand pre-commit.Two decisions worth reviewing
The indent is two, not four. Four is unreachable, not merely unchosen.
indentationdefaults tospaces: consistent, so every step in the file must bethe same width, and a
-item prefix already costs two columns for the mappingkey inside a sequence item. PyYAML's
indent=4pads the dash to- key:andtrips
hyphens; re-indenting by hand to- key:tripsindentationinstead, at 279 findings. Measured all four variants before settling.
The config is committed rather than left user-level. CI runs
task check,and a GitHub runner has no
~/.config/yamllint/config— without a repositoryconfig the stage would fall back to yamllint's upstream defaults,
line-length: 80above all, and fail on nearly every file while passing locally..yamllintalso has to name
.pagesexplicitly inyaml-files, since yamllint selects byfilename and would otherwise skip the 65 extensionless sources that
nav.ymlisgenerated from.
Escape hatches, both deliberate
A few nav entries pair a long title with a long path and exceed 120 columns.
YAML folds plain scalars only at internal spaces, never after the
:, so theycannot be wrapped — the generated header carries a
yamllint disable rule:line-lengthdirective rather than relaxing the rule repo-wide. Likewise.github/ISSUE_TEMPLATE/bug.ymlgets adisable-line: the URL alone exceeds thelimit, and a fold inside a folded scalar becomes a space and would break the
Markdown link.
Verification
yamllint .exit 0 · 53 unit tests pass ·dec-tool build-navigation --checkin sync ·
poetry check --lockclean · zensical still resolvesINHERIT: nav.ymlto all 9 sections ·nav.ymlparses equal to the pre-change file.🤖 Generated with Claude Code
https://claude.ai/code/session_014iAoGhmhEkcj4rAfGd7d8Z