Skip to content

chore: benchmark workflow - #238

Draft
syi0808 wants to merge 1 commit into
noahbald:mainfrom
syi0808:chore/bench-report-action
Draft

chore: benchmark workflow#238
syi0808 wants to merge 1 commit into
noahbald:mainfrom
syi0808:chore/bench-report-action

Conversation

@syi0808

@syi0808 syi0808 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Fixes

Features

Breaking changes

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we stick to just the nu file?

@@ -1,15 +1,51 @@
def main [] {
# Files are picked from https://svgo.dev/docs/plugins/sortAttrs/ docs

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you please preserve this comment

CARGO_TERM_COLOR: always

jobs:
bench-base:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think instead of running base-bench for every PR, we should be able to get cached results of the latest merged PR.

HEAD_RESULTS: ${{ needs.bench-head.outputs.results }}
BENCHMARK_THRESHOLD_PERCENT: "5"
run: |
node .github/scripts/criterion-report.mjs report > benchmark-report.md

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it would be simpler to use the stdout generated by criterion as-is; one less thing to maintain.

@syi0808
syi0808 marked this pull request as draft July 7, 2026 01:09
@syi0808

syi0808 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, this pr was opened by mistake. I meant to target the main branch of my forked repo, but i accidentally selected wrong branch.
Thanks for the all reviews, And i will check them.

@noahbald
noahbald force-pushed the main branch 2 times, most recently from 51d5b96 to 7d72b08 Compare July 23, 2026 02:12
@noahbald
noahbald force-pushed the main branch 3 times, most recently from aec012b to 73231ea Compare August 10, 2026 05:06
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.

2 participants