Fix #132: Correct README.md and CONFIG.md documentation drift#149
Fix #132: Correct README.md and CONFIG.md documentation drift#149mcurrier2 wants to merge 3 commits into
Conversation
README.md fixes: - Fix clock source: "nix crate" -> "direct libc::clock_gettime syscall" - Remove non-existent --no-one-way flag from examples - Remove non-existent "cargo bench" section (no bench targets exist) - Fix dashboard examples: -o is a file path not directory, use -m not --mechanism CONFIG.md fixes: - Fix --output-file description: optional flag, not default filename - Replace --streaming-output with correct --streaming-output-json/csv - Add missing CLI flags to Advanced Options table: --quiet, --log-file, --pmq-priority, --send-delay, --include-first-message, --blocking, --shm-direct, --server, --client - Remove fictional "Configuration File" section (--config flag does not exist) — replaced with shell alias examples - Remove fictional environment variables (IPC_BENCHMARK_OUTPUT_DIR, IPC_BENCHMARK_CONFIG, RUST_LOG) that the application does not use - Remove non-existent --no-one-way and --no-round-trip from test scenario examples - Remove non-existent --dry-run from validation commands - Fix RUST_LOG=debug -> -v flag in debugging examples - Fix dashboard section: -o <dir> -> -o <file>, --mechanism -> -m AI-assisted-by: Claude Opus 4 (claude-sonnet-4-20250514) Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR corrects documentation drift in CONFIG.md and README.md by aligning documented CLI flags, dashboard command examples, performance debugging steps, and architecture details (SHM platform support, clock source, concurrency behavior) with the actual ipc-benchmark implementation. No code entities are changed. ChangesDocumentation Accuracy Fixes
Estimated code review effort: 2 (Simple) | ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📈 Changed lines coverage: N/A (0/0)✅ All new and modified lines are covered by tests!📊 Code Coverage Summary
|
README.md: - Fix broken markdown fence: missing closing ``` before "Understanding Test Types" section header (line 548) caused all subsequent markdown to render inside a code block - Fix platform support: ring buffer uses libc::pthread_* which is Unix-only, not "All platforms (Linux, macOS, Windows, BSD)" - Fix "File Output Expectations": remove fictional auto-generated filenames (sharedmemory_1024_summary.json), replace with accurate example showing user-specified filenames - Add PMQ to concurrency support table (was missing entirely) - Fix concurrency behavior description: "Multi-threaded workers" not "Simulated concurrency (sequential tests)" CONFIG.md: - Fix dashboard troubleshooting: -o results/ -> -o results.json (file path, not directory) AI-assisted-by: Claude Opus 4 (claude-sonnet-4-20250514) Co-authored-by: Cursor <cursoragent@cursor.com>
- Replace "https://github.com/your-org/ipc-benchmark.git" with actual repo URL (redhat-performance/rusty-comms) - Fix "cd ipc-benchmark" to "cd rusty-comms" to match repo name - Remove dead link to non-existent CHANGELOG.md, replace with note about git tags and GitHub releases AI-assisted-by: Claude Opus 4 (claude-sonnet-4-20250514) Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
100-117: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the SHM intro with the Unix-only bullet.
The opening sentence still says the ring-buffer path has cross-platform support, but the updated characteristics now mark it Unix-only. That contradiction makes the section harder to trust; please rewrite the intro so it matches the new platform matrix.
🤖 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 `@README.md` around lines 100 - 117, The SHM section intro currently contradicts the updated platform note in the Ring Buffer description by claiming cross-platform support. Update the opening description in the benchmark suite’s shared memory overview so it matches the Unix-only characterization used in the Ring Buffer characteristics, and keep the wording consistent with the default async path distinction.
🤖 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 `@CONFIG.md`:
- Around line 48-53: The verbosity docs are inconsistent and the
advanced-options table has an extra cell in the `--verbose` row, so fix the
table shape in `CONFIG.md` and update both the `--verbose` entry and the later
note to match the CLI behavior in the verbose handling logic: `-v` = info, `-vv`
= debug, and `-vvv` = trace. Use the `--verbose` row in the advanced-options
table and the later verbosity note together so both places describe the same
levels and repeatable flag behavior.
---
Outside diff comments:
In `@README.md`:
- Around line 100-117: The SHM section intro currently contradicts the updated
platform note in the Ring Buffer description by claiming cross-platform support.
Update the opening description in the benchmark suite’s shared memory overview
so it matches the Unix-only characterization used in the Ring Buffer
characteristics, and keep the wording consistent with the default async path
distinction.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: a9890851-0a01-4559-aa8a-2ac70b30dd37
📒 Files selected for processing (2)
CONFIG.mdREADME.md
| | `--streaming-output-json` | String | - | JSON file for streaming per-message results | | ||
| | `--streaming-output-csv` | String | - | CSV file for streaming per-message results | | ||
| | `--continue-on-error` | Boolean | `false` | Continue running if one test fails | | ||
| | `--verbose` | `-v` | Boolean | `false` | Enable verbose output | | ||
| | `--verbose` | `-v` | Flag | *(repeatable)* | Increase log verbosity (-v=DEBUG, -vv=TRACE) | | ||
| | `--quiet` | `-q` | Boolean | `false` | Suppress stdout output | | ||
| | `--log-file` | String | `ipc_benchmark.log` | Log file path (or "stderr") | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the verbosity docs in both spots.
The advanced-options table has an extra cell in the --verbose row, so Markdown will render it incorrectly, and both this row and the later note still map -v/-vv to DEBUG/TRACE. The CLI actually uses -v = info, -vv = debug, and -vvv = trace, so please normalize the table shape and the text together.
Also applies to: 114-115
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 51-51: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 52-52: Table column count
Expected: 4; Actual: 5; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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 `@CONFIG.md` around lines 48 - 53, The verbosity docs are inconsistent and the
advanced-options table has an extra cell in the `--verbose` row, so fix the
table shape in `CONFIG.md` and update both the `--verbose` entry and the later
note to match the CLI behavior in the verbose handling logic: `-v` = info, `-vv`
= debug, and `-vvv` = trace. Use the `--verbose` row in the advanced-options
table and the later verbosity note together so both places describe the same
levels and repeatable flag behavior.
Source: Linters/SAST tools
📈 Changed lines coverage: N/A (0/0)✅ All new and modified lines are covered by tests!📊 Code Coverage Summary
|
1 similar comment
📈 Changed lines coverage: N/A (0/0)✅ All new and modified lines are covered by tests!📊 Code Coverage Summary
|
Summary
Key Fixes
Test plan
ipc-benchmark --helpCloses #132
Made with Cursor