⏺ All doc drift corrections are complete. Here's a summary of what wa…#32
Conversation
…s updated: ┌──────┬─────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────────────┐ │ Task │ File(s) │ Change │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #9 │ README.md │ Updated workspace structure, crate descriptions, status (906 tests, phases 1-13) │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #10 │ docs/README.md │ Mirrored root README — all 7 crates, current status, build instructions │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #11 │ docs/reference/glossary.md │ Expanded from 25 vm-iucv entries to 50+ covering all crates │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #12 │ docs/cms-core/overview.md │ New page: file system, CommandProcessor, trait seams, execute() flow, abbreviations │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #13 │ docs/cms-spool/overview.md │ New page: devices, classes, commands, SpoolManager, backend trait │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #14 │ docs/cms-pipelines/overview.md │ New page: syntax, built-in stages, two-pass executor, multi-stream, RC codes │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #15 │ docs/cms-machine/overview.md │ New page: handler, console architecture, BATCH_DONE, subsystem wiring, CLI │ ├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤ │ #16 │ docs/SUMMARY.md, docs/quickstart.md │ Added 4 new guide sections to nav, added CMS machine quickstart │ └──────┴─────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────────────────┘
|
PR 32 (myspec-docs) is documentation-only -- no Rust source files changed. The REXX code review criteria (correctness, arithmetic, error handling, safety, tests) do not apply to this diff. I reviewed for documentation accuracy against the codebase instead. See detailed findings in my next comment. |
|
Confirmed accurate: workspace crate list matches Cargo.toml; ExecHandler/SmsgSender/ExtCommandHandler trait seams match crates/cms-core/src/command.rs; abbreviation table in cms-core/overview.md matches CMS_COMMAND_TABLE at command.rs:161 (all 10 entries correct); built-in pipeline stages (literal, console, locate, nlocate) match create_stage factory at cms-pipelines/src/stages/mod.rs:13-18; removing --features rexx from build commands is correct since rexx is the default feature in cms-machine/Cargo.toml:9; dropping --all-features from cargo test is correct for the same reason. |
|
One factual error introduced by this PR: both README.md and docs/README.md change the workspace diagram root label from patch-cms/ to patch-xedit/. The GitHub repository is navicore/patch-cms and there is no patch-xedit label anywhere in Cargo.toml or project files. The old patch-cms/ label matched the repo name; patch-xedit/ does not correspond to anything current and will confuse readers. This appears to be a copy-paste artifact from an earlier project name. The diagram root should remain patch-cms/. No other issues found -- the new overview pages for cms-core, cms-spool, cms-pipelines, and cms-machine are detailed and consistent with the source code. |
|
PR Review: This PR contains only documentation and configuration changes, no Rust source files were modified. |
|
PR Review continued. Confirmed accurate: Trait names ExecHandler, SmsgSender, ExtCommandHandler, NoExecHandler, NoSmsgSender, NoExtCommands match crates/cms-core/src/command.rs. Default no-op return codes RC=28 for NoExecHandler and NoSmsgSender confirmed at lines 51 and 68. provide_state/retrieve_state and provide_smsg_sender/retrieve_smsg_sender pairs in ExecHandler confirmed at lines 20-41. Abbreviation table in docs/cms-core/overview.md matches CMS_COMMAND_TABLE exactly at command.rs lines 161-172. Test count 906 tests passing verified by cargo test --workspace: 117+24+7+88+143+77+402+48 = 906. |
|
PR Review - Issue found in docs/cms-machine/overview.md: The multiple --disk CLI example is wrong. Two problems against crates/cms-machine/src/main.rs: (1) --disk is single-valued. parse_args() declares let mut disk_path = None (line 25) and overwrites it on each --disk occurrence (line 41). Passing --disk /tmp/d1 --disk /tmp/d2 silently ignores /tmp/d1 and mounts only /tmp/d2. (2) Only the A-disk is read-write. The code mounts disk/a with AccessMode::ReadWrite (line 98) and auto-discovers disk/b through disk/z with AccessMode::ReadOnly (line 108). The doc claim in read-write mode applies only to A. Correct mental model: pass a single --disk path as a container directory; a/ is the A-disk (read-write), and any existing b/ through z/ subdirectories are auto-mounted read-only. No other inaccuracies found. The new overview pages for cms-core, cms-spool, cms-pipelines, and the glossary expansions accurately reflect the implementation. |
…s updated:
┌──────┬─────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────────────┐
│ Task │ File(s) │ Change │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #9 │ README.md │ Updated workspace structure, crate descriptions, status (906 tests, phases 1-13) │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #10 │ docs/README.md │ Mirrored root README — all 7 crates, current status, build instructions │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #11 │ docs/reference/glossary.md │ Expanded from 25 vm-iucv entries to 50+ covering all crates │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #12 │ docs/cms-core/overview.md │ New page: file system, CommandProcessor, trait seams, execute() flow, abbreviations │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #13 │ docs/cms-spool/overview.md │ New page: devices, classes, commands, SpoolManager, backend trait │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #14 │ docs/cms-pipelines/overview.md │ New page: syntax, built-in stages, two-pass executor, multi-stream, RC codes │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #15 │ docs/cms-machine/overview.md │ New page: handler, console architecture, BATCH_DONE, subsystem wiring, CLI │
├──────┼─────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────────────┤
│ #16 │ docs/SUMMARY.md, docs/quickstart.md │ Added 4 new guide sections to nav, added CMS machine quickstart │
└──────┴─────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────────────────┘