Skip to content

docs: update README#75

Open
sayalibhavsar wants to merge 1 commit intomainfrom
docs/update-readme
Open

docs: update README#75
sayalibhavsar wants to merge 1 commit intomainfrom
docs/update-readme

Conversation

@sayalibhavsar
Copy link
Copy Markdown

Description

changes made to coremark-wrapper documentation

Before/After Comparison

Changes include a template followed across all wrapper

Solves issue: #74
Relates to JIRA: RPOPC-937

@github-actions
Copy link
Copy Markdown

This relates to RPOPC-937

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Comprehensive README documentation for CoreMark wrapper

📝 Documentation

Grey Divider

Walkthroughs

Description
• Completely rewrote README with comprehensive documentation structure
• Added detailed command-line options with descriptions and examples
• Documented full workflow of coremark_run script with 10 steps
• Included architecture support, dependencies, and troubleshooting sections
• Added practical examples for common use cases and thread scaling modes
Diagram
flowchart LR
  A["Old README<br/>Basic description"] -->|Expand & Restructure| B["New README<br/>Comprehensive docs"]
  B --> C["Command-line<br/>Options"]
  B --> D["Workflow<br/>Steps"]
  B --> E["Examples &<br/>Use Cases"]
  B --> F["Dependencies &<br/>Architecture"]
  B --> G["Output Files &<br/>Troubleshooting"]
Loading

Grey Divider

File Changes

1. README.md 📝 Documentation +276/-24

Complete README restructure with comprehensive documentation

• Replaced minimal 32-line README with comprehensive 284-line documentation
• Added structured sections with markdown headers for navigation
• Expanded command-line options with detailed descriptions and usage patterns
• Documented complete 10-step workflow of coremark_run script execution
• Added examples section with 6 practical use cases and combinations
• Included thread scaling modes (default, powers of 2, linear increment) with detailed explanations
• Added dependencies section with OS-specific package requirements
• Included output files reference, return codes, architecture support, and troubleshooting tips

README.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 21, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Usage shows wrong flag 🐞 Bug ≡ Correctness
Description
coremark_run --usage prints the scaling option as --powers_2s, but the script only parses
--powers_2; users following the built-in usage output will hit “option not found” and exit. The
README now correctly documents --powers_2, making this mismatch more likely to be noticed and
confusing.
Code

README.md[R21-27]

+CoreMark Options:
+  --commit <n>: Git commit or tag to use. Default is v1.01.
+  --cpu_add <n>: Starting at CPU count of 1, add this number of CPUs to each run.
+      Useful for linear thread scaling tests (e.g., --cpu_add 4 runs at 1, 5, 9, ... up to max CPUs).
+  --powers_2: Starting at 1, run the number of CPUs by powers of 2.
+      Runs at 1, 2, 4, 8, 16, ... up to max CPUs.
+      Cannot be used together with --cpu_add.
Evidence
The README documents --powers_2, and the script’s argument parsing also recognizes --powers_2;
however, the usage() function still prints --powers_2s, which is not accepted by the getopt/case
handling. This creates conflicting guidance between README.md and ./coremark_run --usage, and
also makes --usage itself misleading.

README.md[21-27]
coremark/coremark_run[75-83]
coremark/coremark_run[318-366]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`coremark_run` prints an invalid option name in its `--usage` output (`--powers_2s`), while the script only accepts `--powers_2`.

### Issue Context
- README now documents `--powers_2`.
- `coremark_run`’s getopt/case logic handles `--powers_2`.
- `coremark_run --usage` still tells users to use `--powers_2s`.

### Fix Focus Areas
- coremark/coremark_run[75-83]
- coremark/coremark_run[318-366]

### Implementation notes
- Update the `usage()` text to advertise `--powers_2` (remove the trailing `s`).
- (Optional but helpful) Accept `--powers_2s` as a backwards-compatible alias by adding it to the no-argument option list and handling it in the case statement, mapping it to `powers_2=1`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@sayalibhavsar sayalibhavsar self-assigned this Apr 21, 2026
@sayalibhavsar sayalibhavsar requested a review from grdumas April 21, 2026 14:53
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