Skip to content

Review the documentation before the 2.3.00 release - #187

Merged
bertysentry merged 3 commits into
mainfrom
docs/pre-release-review-2.3.00
Sep 29, 2026
Merged

bertysentry merged 3 commits into
mainfrom
docs/pre-release-review-2.3.00

Conversation

@bertysentry

Copy link
Copy Markdown
Contributor

A pre-release review of the whole site (src/site/markdown/*.md, site.xml) and README, to make the 2.3.00 docs accurate, complete, consistent and no longer than needed. The pages come out slightly shorter overall (+526 / −530).

How it was reviewed

Twelve reviewers each covered either a group of pages, checked against the source, or a cross-cutting angle: API coverage, links and structure, a newcomer reading in menu order, and compiling every code example. That produced 361 findings. A skeptical verifier per file then re-checked each finding in the code: 179 were confirmed, 30 refuted and 152 duplicates. 176 are applied here, and every edit was re-checked per file and read in full.

What changed

Wrong statements corrected

  • WQL row values are always strings, and a WMI NULL comes back as an empty string (get() is not a typed value).
  • WinRMFaultException.getFaultCode() returns a String that can be null.
  • Only an HTTP 401 becomes WinRMAuthenticationException. A client-side Kerberos failure (KDC, SPN, clock skew) is a plain WinRMClientException (authentication, timeouts-and-errors, preparing-the-host).
  • legacy.md said the output code page is detected before each call. That has not been true since 2.0.00.
  • tls.md said --https-permissive sets the insecure system property. It calls trustAllCertificates().
  • The PowerShell section assumed Windows PowerShell 5.x, but Windows Server 2008 R2 / Windows 7 run 2.0.
  • The waitFor(Duration) comment suggested code that does not compile (it returns a boolean).
  • On an empty result, WQL columns are lower case, and SELECT * columns are alphabetical.
  • cli.md recommended pairing Basic with --https-permissive, which hands the password to any impostor host.

Missing coverage added

  • index.md gets a Client options table: every builder option with its default. wql.md and commands.md already pointed readers to "the Overview for the builder options", which listed none. It also says a client may be shared between threads, with operations serialized.
  • authentication.md: with Kerberos, the DOMAIN\ part of the user name is ignored (use user@REALM). With ticketCache(...), credentials(...) is still required and must name the cached principal.
  • migrating-from-1x.md gets the breaking changes it left out: Kerberos over HTTP is rejected even as a fallback, output is UTF-8, legacy result collections are unmodifiable, and the transfer directory and share were renamed.
  • A few short notes where users get stuck:
    • calling the same client while consuming a stream waits forever;
    • a UNC upload destination is a second hop;
    • JDKs refuse TLS 1.0/1.1;
    • Git Bash rewrites /switch arguments;
    • the CLI's wql always queries ROOT\CIMV2.

Structure and consistency

  • File Transfers covers uploads and how downloads work. Remote Files no longer repeats the download section.
  • CLI option tables on topic pages (authentication.md, tls.md) become pointers to cli.md, which stays the single source of truth.
  • Duplicated exception tables and implementation-rationale passages are trimmed, e.g. the PowerShell script-size bullet, the BOM paragraph, read performance, and the non-admin WMI steps.
  • Version notation is now 2.0.00 everywhere. The old Doxia-1 anchors (#Privileges and the like) are now lowercase-hyphen. "Platform trust store" becomes the JDK's cacerts.

CLI help and Javadoc

  • --help printed DOMAIN\\user with two backslashes. Copied as-is, that user name fails to authenticate.
  • "Connection options" also listed non-connection options; the header now reads "Options (before the subcommand)".
  • The -i line implied stdin is only forwarded with -i. Forwarding is automatic when stdin is redirected; -i forces it.
  • Stale Javadoc fixed:
    • CommandRequest.stdinCharset;
    • the WinRMCommandExecutor example, which named a class that does not exist and passed the wrong arguments;
    • WqlResult.columns();
    • WindowsRemoteCommandResult: execution time is in seconds, and the missing @return descriptions are added.

README
Only wrong or stale content is corrected: the feature list, the release instructions (Central Portal with auto-release, not OSSRH/Nexus staging), and two outdated option paragraphs, now replaced by a link to the site.

For the reviewer

  • site.xml is unchanged: the menu was reviewed and found fine.
  • The review also found three code bugs. They are deliberately left out of this docs PR:
    1. The streaming terminals (stream(), start(), openStream()) can leak a raw IllegalStateException for protocol failures (LightWinRMService.callStreaming). The docs keep promising WinRMClientException, so the code should be fixed.
    2. WQL array properties keep only their last element (WsmanClient.collectRows). wql.md does not mention arrays until this is fixed.
    3. A multi-line WHERE clause is rejected locally (WmiHelper pattern without DOTALL).
  • mvn clean verify site passes: 322 tests, no checkstyle/PMD/SpotBugs findings. A script over the generated site found no broken internal link or anchor.

🤖 Generated with Claude Code

A full pass over src/site/markdown, site.xml and README.md, checked
against the code: every finding was verified in the source before being
applied.

- Correct wrong statements: WQL row values are always strings (a WMI
  NULL is an empty string), getFaultCode() returns a String, client-side
  Kerberos failures are a plain WinRMClientException, the legacy API no
  longer detects the output code page, --https-permissive does not set
  the insecure system property, PowerShell can be 2.0 on old hosts, and
  Basic must not be combined with --https-permissive.
- Document what was missing: the builder options and their defaults (a
  new "Client options" table on the overview, which two pages already
  pointed to), thread sharing, the Kerberos principal and ticket-cache
  rules, and the 1.x breaking changes the migration page left out.
- Tighten the structure: File Transfers covers uploads and how downloads
  work, Remote Files no longer repeats it; CLI option tables on topic
  pages become pointers to cli.md; repeated exception tables and
  implementation rationale are trimmed.
- Make it consistent: 2.0.00 version notation, lowercase anchors, the
  JDK trust store wording.
- Fix the --help text (DOMAIN\user printed with two backslashes, the
  "Connection options" header, the -i line) and stale Javadoc.
- README: current feature list and release instructions.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T17:56:49.632810Z 9ce8e6d Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ad7cb2cb7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/site/markdown/authentication.md Outdated
Comment thread src/site/markdown/file-transfers.md Outdated
Comment thread src/site/markdown/wql.md
- authentication.md: the client strips all whitespace from the user
  name (WinRMEndpoint), so a qualified Basic name is not sent
  unchanged.
- file-transfers.md: the 30-day purge only covers the transfer-and-run
  directory; leftovers next to an uploadFile(...) destination stay.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@bertysentry

Copy link
Copy Markdown
Contributor Author

@codex please review again

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c3cbb32b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/site/markdown/commands.md
Comment thread src/site/markdown/cli.md Outdated
Only arguments containing whitespace or a double quote are quoted, so
the remote cmd.exe interprets &, |, <, >, ^ and % in the others. Say
so instead of promising that each argument stays one word.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@bertysentry

Copy link
Copy Markdown
Contributor Author

@codex please review again

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 9ce8e6d8e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@bertysentry
bertysentry merged commit 0cc2701 into main Sep 29, 2026
5 checks passed
@bertysentry
bertysentry deleted the docs/pre-release-review-2.3.00 branch September 29, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant