Skip to content

ONP-4123: Address post-merge review comments on MRO benchmarks page - #10772

Merged
soulchips merged 1 commit into
mainfrom
ONP-4123/address-post-merge-review-comments
Sep 22, 2026
Merged

soulchips merged 1 commit into
mainfrom
ONP-4123/address-post-merge-review-comments

Conversation

@soulchips

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #10736. Addresses rosieyohannan's review, which was submitted after the PR merged.

  • Fix semicolon to period in intro paragraph (; all. All)
  • Expand jobs/min to jobs per minute in test setup
  • Split em-dash sentence into two sentences and move "Both resource classes shared the same node group." to before the table
  • Move p50/p90/p99 definitions from test setup to under the Increasing-load results heading, reformatted as an AsciiDoc definition list
  • Convert inline resource class spec lines (Resource class: ... — VM, maxReplicas=N) to bullet lists, applied consistently to all four subsections (large/small increasing-load, large/small burst)

Test plan

  • Vale passes with zero errors

- Fix semicolon to period in intro paragraph (rosieyohannan)
- Expand "jobs/min" to "jobs per minute" (rosieyohannan)
- Split em-dash sentence and move "Both resource classes..." before table (rosieyohannan)
- Move p50/p90/p99 definitions under Increasing-load results as a definition list (rosieyohannan)
- Convert resource class spec lines to bullet lists across all four subsections (rosieyohannan)
@linear-code

linear-code Bot commented Sep 22, 2026

Copy link
Copy Markdown

ONP-4123

@circleci-factory-bot circleci-factory-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FAIL 🔴 author-membership ⚙️ author not in allow-list

soulchips is not in the allow-listed users; allowed: rosieyohannan

PASS 🟢 impact-matches-intent ✨ Editorial docs change matches stated intent

Editorial-only edits to machine-runner-orchestrator-performance-benchmarks.adoc (punctuation, "jobs/min"→"jobs per minute", sentence split/reorder, inline→bullet/definition lists). No exported entity, config value, or customer-gated surface changes; stated intent matches the diff and no consumers break.

SKIP ⚪ 6 skipped check(s)
SKIP ⚪ api-auth-and-design ⚙️ no V3/auth signals in diff

no V3 or auth signals in 1 changed file(s): docs/guides/modules/execution-runner/pages/machine-runner-orchestrator-performance-benchmarks.adoc

SKIP ⚪ agents-md-respect ✨ Style fixes comply with AGENTS.md punctuation rules

The diff replaces semicolons and em-dashes with periods/colons/bullet lists, expands "jobs/min" to "jobs per minute," and reformats percentile definitions as a definition list — all directly aligned with AGENTS.md rules (avoid semicolons, avoid dashes to split sentences or separate item/description, capitalize and punctuate list items). No applicable instruction-file rule is violated.

SKIP ⚪ git-hygiene ⚙️ git history is clean

no merge commits, and no problems with the commit messages

SKIP ⚪ go-best-practices ⚙️ no Go changes to review

no reviewable Go changes among 1 changed file(s)
ignoring 1 non-Go files: docs/guides/modules/execution-runner/pages/machine-runner-orchestrator-performance-benchmarks.adoc

SKIP ⚪ reduces-risk ⚙️ non-test files changed

change touches 1 non-test file(s): docs/guides/modules/execution-runner/pages/machine-runner-orchestrator-performance-benchmarks.adoc

SKIP ⚪ server-compat ⚙️ no new required config keys

no new env.MustGet calls added in this PR

🤖 factory-bot · codeowner mode · reviewed @ 2026-09-22T16:45:54Z · re-request a review to re-run
[trace:63718f7] [bot 7479d8b] [config dc281d7] [target cc5d40a]

@soulchips
soulchips merged commit 7ab3883 into main Sep 22, 2026
4 checks passed
@soulchips
soulchips deleted the ONP-4123/address-post-merge-review-comments branch September 22, 2026 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants