-
Notifications
You must be signed in to change notification settings - Fork 2
DOC-1387 Add Kubernetes automations to the CLI #103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce significant enhancements and new features to the documentation tooling scripts. The main CLI script ( Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (doc-tools.js)
participant GitHub
participant LocalFS
participant UtilityModules
User->>CLI (doc-tools.js): Run "generate helm-spec" or "generate crd-spec"
CLI (doc-tools.js)->>UtilityModules: Verify dependencies (requireTool, requireCmd, etc.)
CLI (doc-tools.js)->>GitHub: (If GitHub source) Clone repo/fetch files
CLI (doc-tools.js)->>LocalFS: Run tool (helm-docs/crd-ref-docs), process files
CLI (doc-tools.js)->>UtilityModules: Convert output (Markdown to AsciiDoc, post-process)
CLI (doc-tools.js)->>LocalFS: Write results, clean up temp files
CLI (doc-tools.js)->>User: Print completion message/instructions
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Docstrings generation was requested by @JakeSCahill. * #103 (comment) The following files were modified: * `bin/doc-tools.js` * `cli-utils/convert-doc-links.js` * `cli-utils/self-managed-docs-branch.js`
Note Generated docstrings for this pull request at #104 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (8)
cli-utils/self-managed-docs-branch.js (3)
56-60
: Document or rethink the hard-coded 2.4 → 24.3 exceptionThe special-case mapping from
v2.4
tov/24.3
(ormain
) is opaque. New
contributors will not know why 2.4 maps to 24.3, and future version bumps may
break silently.Action items
• Add a code comment that links to the policy/decision doc.
• Consider externalising the mapping to a JSON or env-driven config so unusual
rules don’t require code changes.
70-72
: Synchronousgit
call blocks the event loop
spawnSync
freezes the entire Node process while the network call to the
remote Git repo runs. For CLI tools invoked in scripts this can add noticeable
latency, and in long-running processes it can starve the event loop.If you don’t truly need strict serial execution, switch to the async variant:
-const ok = spawnSync('git', ['ls-remote', '--exit-code', '--heads', repo, ref], { - stdio: 'ignore' -}).status === 0 +const ok = (await new Promise(r => + spawn('git', ['ls-remote', '--exit-code', '--heads', repo, ref], { stdio: 'ignore' }) + .on('exit', code => r(code === 0)) +))(Or reuse an existing git-API package for cleaner code.)
41-45
: Minor: broaden accepted prefixesYou strip only
operator/
andrelease/
. Tags likehelm/
orchart/
would
fail even though they are version-qualified strings. Consider making the prefix
list configurable or defaulting to “trim anything before the firstv
”.cli-utils/convert-doc-links.js (3)
18-22
: Label-extraction regex is too greedy
/^(.*)\[([^\]]+)\]$/
will treat the last[
as the delimiter, so a URL
containing a real[
in its path (rare, but legal) mis-parses.
Use a non-greedy match or limit the character class before the label:-const mLabel = input.match(/^(.*)\[([^\]]+)\]$/); +const mLabel = input.match(/^(.*?)\s*\[([^\]]+)\]$/);Alternatively split on the first
[
from the right with
const idx = input.lastIndexOf('[')
.
31-33
: Host validation blockswww.docs.redpanda.com
& future sub-domainsThe strict regex only allows the bare domain. If marketing switches to
www.docs.redpanda.com
or a regional CNAME, the converter breaks.-if (!/docs\.redpanda\.com$/.test(url.hostname)) { +if (!/(\.)?docs\.redpanda\.com$/.test(url.hostname)) {You might also want to allow staging domains behind a feature flag.
45-53
: Edge-case: root-level pages lose their moduleWhen the path is just a single segment (e.g.
/legal/
),moduleName
and the
resulting file name are identical (legal.adoc
). That’s fine if every module
has a root page named like itself; otherwise, you might want to default to
index.adoc
.Consider:
const fileName = (pagePath || 'index') + '.adoc';package.json (1)
3-3
: Version bump acknowledged—don’t forget CHANGELOG
"version": "4.5.0"
looks good. Ensure the accompanying CHANGELOG or release
notes capture the new Kubernetes CLI features so downstream consumers know what
changed.bin/doc-tools.js (1)
548-552
: Minor readability tweak: optional chaining simplifies the null-check-const inDocs = pkg.name === 'redpanda-docs-playbook' - || (pkg.repository && pkg.repository.url.includes('redpanda-data/docs')); +const inDocs = pkg.name === 'redpanda-docs-playbook' + || pkg.repository?.url?.includes('redpanda-data/docs');Using optional chaining conveys intent and eliminates the manual
&&
guard.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
bin/doc-tools.js
(7 hunks)cli-utils/convert-doc-links.js
(1 hunks)cli-utils/self-managed-docs-branch.js
(1 hunks)package.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
bin/doc-tools.js (3)
cli-utils/convert-doc-links.js (4)
require
(1-1)p
(36-39)moduleName
(49-49)url
(25-25)cli-utils/self-managed-docs-branch.js (5)
require
(3-3)repo
(68-68)ref
(69-69)ok
(70-72)url
(10-10)tools/fetch-from-github.js (2)
path
(2-2)fs
(1-1)
🪛 Biome (1.9.4)
cli-utils/self-managed-docs-branch.js
[error] 21-21: Invalid typeof
comparison value: this expression is not a string literal
not a string literal
(lint/suspicious/useValidTypeof)
bin/doc-tools.js
[error] 567-568: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - docs-extensions-and-macros
- GitHub Check: Header rules - docs-extensions-and-macros
- GitHub Check: Pages changed - docs-extensions-and-macros
Docstrings generation was requested by @JakeSCahill. * #103 (comment) The following files were modified: * `bin/doc-tools.js` * `cli-utils/convert-doc-links.js` * `cli-utils/self-managed-docs-branch.js` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This pull request introduces two new utility functions for handling documentation-related tasks and updates the package version to reflect these additions. The key changes include a function to convert documentation URLs into Antora xref resource IDs, another to determine the appropriate documentation branch based on an operator tag, and a version bump in
package.json
.New CLI commands
Added two new CLI commands for generating the Helm spec docs and the CRD docs for Kubernetes.
New utility functions:
cli-utils/convert-doc-links.js
: AddedurlToXref
function to convertdocs.redpanda.com
URLs into Antora xref resource IDs, preserving optional labels. This includes validation and parsing of the URL structure.cli-utils/self-managed-docs-branch.js
: AddedfetchRemoteAntoraVersion
to retrieve the latest documented Self-Managed version from a remoteantora.yml
file, anddetermineDocsBranch
to decide the appropriate documentation branch based on an operator tag and the Antora version. Includes branch existence verification.Version update:
package.json
: Updated the package version from4.4.2
to4.5.0
to reflect the addition of new features.Test the new commands
Clone this repo or pull the latest:
Check out the branch:
Run the commands: