Update 2026#68
Conversation
|
Test deployment available at https://callumwalley.github.io/mkdocs-demo-deploy/nesi/research-developer-cloud/update-2026 See all deployed demo sites |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the documentation build system and user interface. Key additions include an ARIA reference checker, a title capitalization linter, and a light/dark mode palette toggle. The documentation theme was also updated with new styles for tutorials, quizzes, and various admonition types. Technical feedback focuses on resolving several redundancies and inefficiencies in the Python check scripts, such as duplicate variable initializations and function definitions. Additionally, corrections are required for CSS selector typos and a logic mismatch between the tag count requirement and its corresponding error message.
|
|
||
| msg_count = {"debug": 0, "notice": 0, "warning": 0, "error": 0} | ||
|
|
||
| msg_count = {"debug": 0, "notice": 0, "warning": 0, "error": 0} |
There was a problem hiding this comment.
Redundant initialization of msg_count. It is already defined on line 25.
| global \ | ||
| input_path, \ | ||
| title_from_h1, \ | ||
| title_from_filename, \ | ||
| title, \ | ||
| meta, \ | ||
| contents, \ | ||
| input_path |
There was a problem hiding this comment.
Redundant input_path in the global statement. It is listed at both line 57 and line 63.
global \
input_path, \
title_from_h1, \
title_from_filename, \
title, \
meta, \
contents| for check in WALKCHECKS: | ||
| in_code_block = not in_code_block if re.match(r"^\s*```\s?\w*$", line) else in_code_block | ||
|
|
||
| _get_nav_tree() | ||
| _run_check(check) |
There was a problem hiding this comment.
Performance improvement: _get_nav_tree() only needs to be called once per line to update the TOC structure. Moving it outside the inner loop avoids redundant calls for each check in WALKCHECKS.
| for check in WALKCHECKS: | |
| in_code_block = not in_code_block if re.match(r"^\s*```\s?\w*$", line) else in_code_block | |
| _get_nav_tree() | |
| _run_check(check) | |
| _get_nav_tree() | |
| for check in WALKCHECKS: | |
| _run_check(check) |
| "message": "Try to include at least 2 'tags'\ | ||
| (helps with search optimisation).", |
There was a problem hiding this comment.
The error message mentions "at least 2 'tags'", but the MIN_TAGS constant was updated to 1 on line 33. The message should be updated to reflect the current requirement.
"message": "Try to include at least 1 'tag' (helps with search optimisation)."| print(m1.group(2)) | ||
| yield {"line": lineno,"message": f"Link '{m1.group(0)}', does not use dynamic slurm version. Use 'https://slurm.schedmd.com/archive/{{{{ config.extra.slurm }}}}/{m1.group(2)}"} | ||
|
|
||
| def dynamic_slurm_link(): |
There was a problem hiding this comment.
The function dynamic_slurm_link is defined twice in this file (once at line 396 and again at line 406). The second definition is redundant and should be merged or removed.
|
|
||
| ALLOWABLE_NOTICES = 5 | ||
|
|
||
| ALLOWABLE_NOTICES = 5 |
There was a problem hiding this comment.
Redundant initialization of ALLOWABLE_NOTICES. It is already defined on line 16.
| .md-typeset .admonition.objectives, | ||
| .md-typeset details.time { | ||
| border-color: var(--reannz-purple) | ||
| } |
There was a problem hiding this comment.
Possible copy-paste error: details.time is used within the objectives style block. It should likely be details.objectives. Additionally, the border-color property is missing a trailing semicolon.
| .md-typeset .admonition.objectives, | |
| .md-typeset details.time { | |
| border-color: var(--reannz-purple) | |
| } | |
| .md-typeset .admonition.objectives, | |
| .md-typeset details.objectives { | |
| border-color: var(--reannz-purple); | |
| } |
| background-color: rgba(241, 128, 0, 0.1); | ||
| } | ||
|
|
||
| .quiz-question > .admonition-titl e::before{ |
There was a problem hiding this comment.
Typo in CSS selector: .admonition-titl e should be .admonition-title.
| .quiz-question > .admonition-titl e::before{ | |
| .quiz-question > .admonition-title::before{ |
|
LGTM I did notice with the nav changes that we lost the index level of those nav items eg all the index.md files |
|
Looks the same to me? |
|
LGTM |
2 participants
Updates checks and dependencies to be inline with main docs.
Fixes #67