Skip to content

docs: update README#60

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

docs: update README#60
sayalibhavsar merged 1 commit intomainfrom
docs/update-readme

Conversation

@sayalibhavsar
Copy link
Copy Markdown
Contributor

Description

changes made to coremark_pro-wrapper documentation

Before/After Comparison

Changes include a template followed across all wrapper

Solves issue: #59
Relates to JIRA: RPOPC-938

@sayalibhavsar sayalibhavsar self-assigned this Apr 21, 2026
@github-actions
Copy link
Copy Markdown

This relates to RPOPC-938

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Comprehensive CoreMark-PRO wrapper documentation overhaul

📝 Documentation

Grey Divider

Walkthroughs

Description
• Comprehensive README restructure with detailed workflow documentation
• Added command-line options reference with all parameters explained
• Included workload descriptions, metrics, and output file specifications
• Added usage examples and troubleshooting guidance
Diagram
flowchart LR
  A["Basic README"] -->|Expand| B["Structured Documentation"]
  B --> C["Command Options"]
  B --> D["Workflow Steps"]
  B --> E["Workload Details"]
  B --> F["Usage Examples"]
  B --> G["Troubleshooting"]
Loading

Grey Divider

File Changes

1. README.md 📝 Documentation +244/-30

Expand README with comprehensive workflow and usage documentation

• Restructured from brief overview to comprehensive 252-line documentation
• Added detailed 10-step workflow explanation covering environment setup, package installation,
 CoreMark-PRO download, workload tuning, build, execution, data collection, result processing,
 verification, and output
• Expanded command-line options section with full parameter descriptions for CoreMark-PRO and
 general test_tools options
• Added workload table describing nine distinct CoreMark-PRO workloads with their computational
 domains
• Included metrics explanation for multi-core iterations, single-core iterations, and scaling ratios
• Added output files section documenting all generated result files and their purposes
• Provided five practical usage examples with explanations
• Added sections on workload tuning mechanics, return codes, architecture support, performance tips,
 and troubleshooting

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. Nonfunctional --tools_git docs 🐞 Bug ≡ Correctness
Description
README documents a --tools_git option to select the test_tools-wrappers repository, but
coremark_pro_run always acquires tools from a hardcoded tools_git URL before any wrapper
parsing, so that documented option cannot affect behavior.
Code

README.md[R40-41]

+  --tools_git <value>: Git repo to retrieve the required tools from.
+      Default: https://github.com/redhat-performance/test_tools-wrappers
Evidence
README advertises a configurable tools repo via --tools_git, but the wrapper hardcodes tools_git
and runs wget/curl/git acquisition immediately. The script only sources general_setup (where such
a generic option might be parsed) later, after acquisition has already happened, and the wrapper’s
own getopt list does not include tools_git at all.

README.md[30-42]
coremark_pro/coremark_pro_run[101-142]
coremark_pro/coremark_pro_run[497-523]

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

### Issue description
`README.md` claims users can set `--tools_git` to control where `test_tools-wrappers` is fetched from. In `coremark_pro_run`, tool acquisition happens immediately using a hardcoded `tools_git` value, before later argument parsing/setup, so this option (as documented) cannot change behavior.

### Issue Context
This is a documentation/behavior mismatch that can mislead users attempting to point the wrapper at a fork/mirror of `test_tools-wrappers`.

### Fix Focus Areas
- README.md[30-42]

### Suggested fix
- Either remove `--tools_git` from the README for this wrapper, or explicitly state it is not currently honored by `coremark_pro_run`.
- (Alternative, code change) If you want `--tools_git` to work, parse it (or read an env var) *before* `attempt_tools_wget/curl/git` and use it to set `tools_git` prior to acquisition.

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


Grey Divider

Qodo Logo

@sayalibhavsar sayalibhavsar requested a review from grdumas April 21, 2026 15:14
@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 b95e86a into main Apr 30, 2026
4 of 6 checks passed
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