NO-JIRA:Fix AsciiDoc documentation generation, add Docker-based spell-check, fix typos#3280
Open
vparfonov wants to merge 5 commits into
Open
NO-JIRA:Fix AsciiDoc documentation generation, add Docker-based spell-check, fix typos#3280vparfonov wants to merge 5 commits into
vparfonov wants to merge 5 commits into
Conversation
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Perl AsciiDoc fixer and Makefile docs-spell-check target, applies AsciiDoc post-processing to generated API docs, and corrects many documentation, template, manifest, test, and comment typos and formatting across the repository. ChangesDocumentation and Tooling Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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
Contributor
Author
|
/retest |
jcantrill
reviewed
May 18, 2026
| 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 |
Contributor
There was a problem hiding this comment.
This probably should be added to either the linter script or lint target
Contributor
Author
There was a problem hiding this comment.
enable misspell linter for Go comments and strings
Contributor
|
/hold |
Contributor
|
/approve |
openshift-ci
Bot
added
the
do-not-merge/hold
Contributor
|
[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
changed the title
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@hack/fix-asciidoc.pl`:
- Around line 63-66: The current regex block (if ($line =~
/^(\|[^|]*\|[^|]*\|)(.*)$/) { ... }) escapes every '|' in $desc, which mangles
any valid columns after the description; instead, split $line into its
pipe-separated fields, target only the description cell (the third field) for
escaping, then rebuild $line by joining the fields with '|' and appending "\n"
so subsequent columns remain untouched (operate on $line and the description
variable rather than globally replacing pipes in the tail).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 20af723c-5426-46ae-8efb-5980f08e7d02
📒 Files selected for processing (55)
.codespellignore.golangci.yamlMakefileREADME.adocapi/logging/v1alpha1/doc.goapi/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.plhack/gen-olm-artifacts.pyhack/lib/build/environment.shhack/lib/build/rpm.shhack/lib/cmd.shhack/lib/log/system.shhack/lib/test/junit.shinternal/api/context/context.gointernal/cmd/functional-benchmarker/config/options.gointernal/cmd/functional-benchmarker/readme.mdinternal/cmd/functional-benchmarker/stats/types.gointernal/collector/aws/credentials.gointernal/controller/observability/clusterlogforwarder_controller.gointernal/generator/helpers/container_path_glob_builder_test.gointernal/generator/url/url.gointernal/generator/vector/api/sinks/prometheus_export.gointernal/generator/vector/filter/apiaudit/doc.gointernal/generator/vector/helpers/helpers.gointernal/generator/vector/helpers/node.gointernal/generator/vector/output/lokistack/suite_test.gointernal/hostedcontrolplane/hostedcontrolplane.gointernal/metrics/logfilemetricexporter/daemonset_test.gointernal/validations/observability/inputs/receivers_test.gomust-gather/collection-scripts/commontest/framework/functional/read.gotest/functional/filters/apiaudit/vrl/filter_test.gotest/functional/filters/multilineexception/multiline_exception_test.gotest/functional/flowcontrol/input_policy_test.gotest/functional/misc/generic_logs_test.gotest/helpers/oc/doc.gotest/helpers/oc/runner.gotest/matchers/log_format.go
✅ Files skipped from review due to trivial changes (45)
- api/observability/v1/filter_types.go
- test/helpers/oc/runner.go
- docs/features/logforwarding/filters/prune-filter.adoc
- docs/architecture/readme.adoc
- .golangci.yaml
- internal/generator/vector/filter/apiaudit/doc.go
- .codespellignore
- hack/lib/test/junit.sh
- hack/lib/log/system.sh
- config/docs/templates/datamodels/asciidoc/members.tpl
- hack/lib/build/rpm.sh
- hack/lib/cmd.sh
- test/framework/functional/read.go
- test/helpers/oc/doc.go
- internal/cmd/functional-benchmarker/readme.md
- test/functional/filters/multilineexception/multiline_exception_test.go
- internal/generator/vector/api/sinks/prometheus_export.go
- api/observability/v1/output_types.go
- internal/hostedcontrolplane/hostedcontrolplane.go
- internal/generator/vector/helpers/node.go
- docs/administration/upgrade/v6.0_changes.adoc
- internal/generator/url/url.go
- internal/validations/observability/inputs/receivers_test.go
- docs/administration/lokistack.adoc
- docs/administration/clusterlogforwarder.adoc
- internal/collector/aws/credentials.go
- api/logging/v1alpha1/doc.go
- internal/generator/vector/helpers/helpers.go
- docs/README.adoc
- internal/api/context/context.go
- internal/cmd/functional-benchmarker/stats/types.go
- bundle/manifests/cluster-logging.clusterserviceversion.yaml
- internal/controller/observability/clusterlogforwarder_controller.go
- README.adoc
- config/manifests/bases/cluster-logging.clusterserviceversion.yaml
- docs/reference/operator/api_logging_v1alpha1.adoc
- bundle/manifests/observability.openshift.io_clusterlogforwarders.yaml
- config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml
- hack/lib/build/environment.sh
- api/observability/v1/input_types.go
- test/functional/flowcontrol/input_policy_test.go
- internal/generator/helpers/container_path_glob_builder_test.go
- api/observability/v1/filter_api_audit_types.go
- internal/metrics/logfilemetricexporter/daemonset_test.go
- hack/gen-olm-artifacts.py
Contributor
|
/retest |
Contributor
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
Contributor
|
/lgtm |
Contributor
Author
|
/test functional-target |
1 similar comment
Contributor
Author
|
/test functional-target |
Contributor
Author
|
/test e2e-target |
Contributor
Contributor
Author
|
/retest-required |
Contributor
Author
|
/test functional-target |
Contributor
Contributor
|
@vparfonov: The following tests failed, say
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. |
Contributor
openshift-ci
Bot
added
the
needs-rebase
- 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>
Replace backtick wrapping of {…} patterns with AsciiDoc passthrough (pass:[…]).
Backticks rendered as inline code markers, showing unwanted characters around
braces in table cells. Using pass:[] prevents attribute substitution without
adding visible formatting characters to the output. Pipes (||) remain escaped
to \\|\\| for table structure preservation.
Also fix datamodel template heading levels to use :: inline syntax instead
of separate subsections, and update datamodel member template to use ====
for consistent hierarchy.
Validated all three generated docs (api_observability_v1.adoc,
api_logging_v1alpha1.adoc, viaq/v1.adoc) with asciidoctor — no errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
|
New changes are detected. LGTM label has been removed. |
openshift-ci
Bot
removed
the
needs-rebase
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
3 participants
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.
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