ci: combine benchmark jobs back into one workflow#9963
Conversation
|
eac42a5 to
7972591
Compare
7972591 to
b985c52
Compare
Merging this PR will degrade performance by 38.34%
Performance Changes
Comparing |
WalkthroughThis PR consolidates eleven separate category-specific benchmark workflow files (benchmark_js.yml, benchmark_json.yml, benchmark_css.yml, benchmark_html.yml, benchmark_graphql.yml, benchmark_grit.yml, benchmark_markdown.yml, benchmark_manifests.yml, benchmark_module_graph.yml, benchmark_configuration.yml, and benchmark_tailwind.yml) into a single unified Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/benchmark.yml (1)
160-171: Trim redundant event checks in job guards.Since Line 160–Line 171 already force all category outputs to
trueforworkflow_dispatch/merge_group, repeating those event checks in every jobifis redundant and harder to maintain.Suggested simplification pattern
- if: github.event_name == 'workflow_dispatch' || github.event_name == 'merge_group' || needs.changes.outputs.js == 'true' + if: needs.changes.outputs.js == 'true'Apply the same pattern to the other category jobs.
Also applies to: 188-188, 232-232, 248-248, 264-264, 280-280, 295-295, 333-333, 347-347, 361-361, 375-375, 389-389
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 160 - 171, The workflow sets all category flags (JS, JSON, CSS, HTML, GRAPHQL, MARKDOWN, GRIT, TAILWIND, CONFIGURATION, MANIFESTS, MODULE_GRAPH) to true when EVENT_NAME is 'workflow_dispatch' or 'merge_group', so remove the redundant event-name checks from individual job "if" guards and rely solely on those category boolean outputs instead; update each job's conditional expressions that currently include "[ \"$EVENT_NAME\" = 'workflow_dispatch' ] || [ \"$EVENT_NAME\" = 'merge_group' ]" (the repeated pattern mentioned for lines 188, 232, 248, 264, 280, 295, 333, 347, 361, 375, 389) to only reference the relevant category flag(s) (e.g., JS, CSS, MARKDOWN, etc.) so the global pre-set covers dispatch/merge_group cases and maintenance is simpler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/benchmark.yml:
- Line 29: The custom runner labels (depot-ubuntu-24.04-arm and
depot-ubuntu-24.04-arm-16) used in the workflow
(.github/workflows/benchmark.yml) will fail actionlint; either add an actionlint
config that allow-lists these labels (create .github/actionlint.yml and include
rules permitting those runner labels) or replace each occurrence of
depot-ubuntu-24.04-arm and depot-ubuntu-24.04-arm-16 with standard GitHub runner
labels (e.g., ubuntu-24.04 or ubuntu-latest) across the workflow so actionlint
recognizes them.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 160-171: The workflow sets all category flags (JS, JSON, CSS,
HTML, GRAPHQL, MARKDOWN, GRIT, TAILWIND, CONFIGURATION, MANIFESTS, MODULE_GRAPH)
to true when EVENT_NAME is 'workflow_dispatch' or 'merge_group', so remove the
redundant event-name checks from individual job "if" guards and rely solely on
those category boolean outputs instead; update each job's conditional
expressions that currently include "[ \"$EVENT_NAME\" = 'workflow_dispatch' ] ||
[ \"$EVENT_NAME\" = 'merge_group' ]" (the repeated pattern mentioned for lines
188, 232, 248, 264, 280, 295, 333, 347, 361, 375, 389) to only reference the
relevant category flag(s) (e.g., JS, CSS, MARKDOWN, etc.) so the global pre-set
covers dispatch/merge_group cases and maintenance is simpler.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 296eeb6a-efaf-4194-a87d-58d90db7cfb6
📒 Files selected for processing (12)
.github/workflows/benchmark.yml.github/workflows/benchmark_configuration.yml.github/workflows/benchmark_css.yml.github/workflows/benchmark_graphql.yml.github/workflows/benchmark_grit.yml.github/workflows/benchmark_html.yml.github/workflows/benchmark_js.yml.github/workflows/benchmark_json.yml.github/workflows/benchmark_manifests.yml.github/workflows/benchmark_markdown.yml.github/workflows/benchmark_module_graph.yml.github/workflows/benchmark_tailwind.yml
💤 Files with no reviewable changes (11)
- .github/workflows/benchmark_manifests.yml
- .github/workflows/benchmark_configuration.yml
- .github/workflows/benchmark_json.yml
- .github/workflows/benchmark_js.yml
- .github/workflows/benchmark_html.yml
- .github/workflows/benchmark_module_graph.yml
- .github/workflows/benchmark_css.yml
- .github/workflows/benchmark_markdown.yml
- .github/workflows/benchmark_grit.yml
- .github/workflows/benchmark_tailwind.yml
- .github/workflows/benchmark_graphql.yml
Summary
Codspeed does not support having runs in different workflows. They must all be in the same workflow.
Generated by gpt 5.4
Test Plan
ci should be green
Docs