NO-JIRA:Fix AsciiDoc documentation generation, add Docker-based spell-check, fix typos#3280
NO-JIRA:Fix AsciiDoc documentation generation, add Docker-based spell-check, fix typos#3280vparfonov wants to merge 3 commits into
Conversation
- Convert Markdown-style headings (#/##) to AsciiDoc syntax (==/===)
in API Go doc comments
- Fix heading level hierarchy in datamodel templates (===== to ====)
- Create hack/fix-asciidoc.pl post-processing script to fix generated
AsciiDoc: strip carriage returns, escape || and {}, join multi-line
table cells, escape stray pipes in description cells
- Add docs-spell-check Makefile target using podman with UBI9 Python
image and codespell, no local tooling installation required
- Add .codespellignore for upstream (fileds) and domain-specific
false positives (notin, coo)
- Fix typos in docs: policiy, similiarly, and others
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@vparfonov: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughAdds a Perl AsciiDoc fixer and Makefile docs-spell-check target, applies AsciiDoc post-processing to generated API docs, and corrects many documentation, comment, manifest, template, and test typos and formatting issues across the repo. ChangesDocumentation and Tooling Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Makefile (1)
103-105: ⚡ Quick winConsider making codespell output visible and adding error handling.
The docs-spell-check target silently installs codespell with
--quiet. While this keeps output clean, it may hide installation errors. Additionally, if codespell finds issues, users might want to see them immediately rather than having to re-run without--quiet.♻️ Make errors visible and improve feedback
.PHONY: docs-spell-check docs-spell-check: - podman run --rm -v $(PWD):/workdir:z registry.access.redhat.com/ubi9/python-312:latest sh -c 'pip install --quiet codespell && codespell /workdir/docs/ -I /workdir/.codespellignore' + podman run --rm -v $(PWD):/workdir:z registry.access.redhat.com/ubi9/python-312:latest sh -c 'pip install codespell >/dev/null && codespell /workdir/docs/ -I /workdir/.codespellignore'This redirects pip's progress output but lets errors through, and codespell results remain visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 103 - 105, The docs-spell-check Makefile target currently hides pip install progress/errors and may also suppress codespell output; update the target (docs-spell-check) command to remove the pip --quiet flag, check the pip install exit status, and ensure codespell runs transparently so its findings are printed; for example, run pip install codespell and if it fails exit with a clear error message, then run codespell /workdir/docs/ -I /workdir/.codespellignore so that codespell output is visible to the user.hack/fix-asciidoc.pl (2)
61-61: ⚡ Quick winClarify the purpose of this regex substitution.
The substitution
s/(\d)-(\d+)([A-Z])/$1-$2 $3/gadds a space between a digit-hyphen-digit sequence and a following uppercase letter (e.g., "1-2A" → "1-2 A"). The purpose of this transformation is not obvious from the surrounding context. Consider adding a comment explaining why this is necessary for AsciiDoc processing.📝 Add clarifying comment
# Escape stray pipe characters inside table description cells foreach my $line (`@result`) { next unless $line =~ /^\|/ && $line !~ /^\|\s*=/; + # Prevent ranges like "1-2A" from being misinterpreted $line =~ s/(\d)-(\d+)([A-Z])/$1-$2 $3/g;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/fix-asciidoc.pl` at line 61, The regex substitution s/(\d)-(\d+)([A-Z])/$1-$2 $3/g in fix-asciidoc.pl is unclear; add an inline comment immediately above the substitution explaining that it inserts a space between a digit-hyphen-digit sequence and a following uppercase letter (e.g., "1-2A" → "1-2 A") and why this normalization is required for AsciiDoc processing (such as preventing token merging or ensuring proper word separation for headings/anchors); reference the exact substitution pattern in the comment so future readers can understand the intent.
8-8: 💤 Low valueConsider checking
close()return values for completeness.While
open()errors are checked withor die, theclose()calls don't verify success. In practice, close failures on read handles are rarely critical, but write handle close failures can indicate data loss (e.g., disk full). For a post-processing script that modifies files in place, checking the write close would be more defensive.🛡️ Check close on write
open($fh, '>', $file) or die "Cannot write to $file: $!"; print $fh `@result`; -close($fh); +close($fh) or die "Failed to close $file: $!";Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/fix-asciidoc.pl` at line 8, The script currently calls close($fh) without checking its return; update the code paths that close write filehandles (the close($fh) calls at the locations noted) to verify the return value and handle failures (e.g., die or warn with the actual $! error) so write-close errors (like disk full) are surfaced; keep existing behavior for read-only closes if you prefer, but ensure the in-place write/temporary file close in fix-asciidoc.pl checks close($fh) and reports/handles errors appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@hack/fix-asciidoc.pl`:
- Line 61: The regex substitution s/(\d)-(\d+)([A-Z])/$1-$2 $3/g in
fix-asciidoc.pl is unclear; add an inline comment immediately above the
substitution explaining that it inserts a space between a digit-hyphen-digit
sequence and a following uppercase letter (e.g., "1-2A" → "1-2 A") and why this
normalization is required for AsciiDoc processing (such as preventing token
merging or ensuring proper word separation for headings/anchors); reference the
exact substitution pattern in the comment so future readers can understand the
intent.
- Line 8: The script currently calls close($fh) without checking its return;
update the code paths that close write filehandles (the close($fh) calls at the
locations noted) to verify the return value and handle failures (e.g., die or
warn with the actual $! error) so write-close errors (like disk full) are
surfaced; keep existing behavior for read-only closes if you prefer, but ensure
the in-place write/temporary file close in fix-asciidoc.pl checks close($fh) and
reports/handles errors appropriately.
In `@Makefile`:
- Around line 103-105: The docs-spell-check Makefile target currently hides pip
install progress/errors and may also suppress codespell output; update the
target (docs-spell-check) command to remove the pip --quiet flag, check the pip
install exit status, and ensure codespell runs transparently so its findings are
printed; for example, run pip install codespell and if it fails exit with a
clear error message, then run codespell /workdir/docs/ -I
/workdir/.codespellignore so that codespell output is visible to the user.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 97bafab6-1f3c-4a06-bc01-35225e2f75ea
📒 Files selected for processing (22)
.codespellignoreMakefileapi/observability/v1/filter_api_audit_types.goapi/observability/v1/filter_types.goapi/observability/v1/input_types.goapi/observability/v1/output_types.gobundle/manifests/cluster-logging.clusterserviceversion.yamlbundle/manifests/observability.openshift.io_clusterlogforwarders.yamlconfig/crd/bases/observability.openshift.io_clusterlogforwarders.yamlconfig/docs/templates/datamodels/asciidoc/members.tplconfig/manifests/bases/cluster-logging.clusterserviceversion.yamldocs/README.adocdocs/administration/clusterlogforwarder.adocdocs/administration/lokistack.adocdocs/administration/upgrade/v6.0_changes.adocdocs/architecture/readme.adocdocs/features/logforwarding/filters/prune-filter.adocdocs/reference/datamodels/viaq/v1.adocdocs/reference/operator/api_logging_v1alpha1.adocdocs/reference/operator/api_observability_v1.adochack/fix-asciidoc.plinternal/generator/vector/api/sinks/prometheus_export.go
|
/retest |
| docs: docs/reference/operator/api_observability_v1.adoc docs/reference/operator/api_logging_v1alpha1.adoc docs/reference/datamodels/viaq/v1.adoc | ||
| .PHONY: docs | ||
|
|
||
| .PHONY: docs-spell-check |
There was a problem hiding this comment.
This probably should be added to either the linter script or lint target
There was a problem hiding this comment.
enable misspell linter for Go comments and strings
|
/hold |
|
/approve |
openshift-ci
Bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@vparfonov: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
vparfonov
changed the title
3 participants
Description
#/##) to AsciiDoc syntax (==/===) in API Go doc comments=====to====)hack/fix-asciidoc.plpost-processing script to fix generated AsciiDoc:||and{},docs-spell-checkMakefile target using podman with UBI9 Python image and codespell.codespellignorefor upstream (fileds) and domain-specific false positives (notin,coo,fileds(a real typo from upstreamk8s.io/apiserver))policiy,similiarly, and othersmisspelllinter for Go comments and strings/cc @Clee2691
/assign @jcantrill
Links
Summary by CodeRabbit