Skip to content

docs: update README#75

Merged
sayalibhavsar merged 1 commit into
mainfrom
docs/update-readme
Apr 30, 2026
Merged

docs: update README#75
sayalibhavsar merged 1 commit into
mainfrom
docs/update-readme

Conversation

@sayalibhavsar
Copy link
Copy Markdown
Contributor

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
@grdumas grdumas added the documentation Improvements or additions to documentation label Apr 28, 2026
Copy link
Copy Markdown
Contributor

@grdumas grdumas left a comment

Choose a reason for hiding this comment

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

LGTM

@grdumas grdumas added the group_review_lgtm Indicates approval after a group review meeting label Apr 28, 2026
@sayalibhavsar sayalibhavsar merged commit b30aab5 into main Apr 30, 2026
4 of 6 checks passed
@sayalibhavsar
Copy link
Copy Markdown
Contributor Author

opened to address removing pbench and latest usage options from general setup(test_tool_wrapper)
#77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation group_review_lgtm Indicates approval after a group review meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants