Skip to content

ci: remove spamoor results from benchmark results per PR#3138

Merged
julienrbrt merged 1 commit intomainfrom
cian/disable-spamoor-benchmarks-being-published
Mar 5, 2026
Merged

ci: remove spamoor results from benchmark results per PR#3138
julienrbrt merged 1 commit intomainfrom
cian/disable-spamoor-benchmarks-being-published

Conversation

@chatton
Copy link
Contributor

@chatton chatton commented Mar 5, 2026

Overview

Remove the reporting of the spamoor results per PR, these will be recorded separately from runs that use more appropriate hardware.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

Warning

Rate limit exceeded

@chatton has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 10 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: faa11bd7-eb97-4deb-8146-4188d1a26a43

📥 Commits

Reviewing files that changed from the base of the PR and between 5a07bc3 and a88badc.

📒 Files selected for processing (1)
  • .github/workflows/benchmark.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cian/disable-spamoor-benchmarks-being-published

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedMar 5, 2026, 9:14 AM

@claude
Copy link
Contributor

claude bot commented Mar 5, 2026

Claude finished @chatton's task in 43s —— View job


Code Review

  • Gather context
  • Review changes
  • Post findings

Summary

This is a focused, clean change to .github/workflows/benchmark.yml that removes the spamoor benchmark results from the publish-benchmarks job. The overall change is straightforward and well-scoped.

What Changed

  • publish-benchmarks needs dependency changed from [evm-benchmark, spamoor-benchmark][evm-benchmark]
  • ~24 lines removed: the spamoor download-artifact + benchmark-action publish steps were dropped from publish-benchmarks
  • The spamoor-benchmark job itself is retained — it still runs and uploads artifacts, just no longer feeds into the publish/tracking step

Observations

Intentional design: The spamoor-benchmark job still runs and uploads its artifact (spamoor_bench.json), which is fine for one-off inspection. However, since nothing now consumes that artifact in publish-benchmarks, it's purely stored as a CI artifact. This is consistent with the pattern already established for erc20-benchmark, which has a # TODO: wire up to publish results once additional tests are in place. comment.

Minor suggestion: For consistency with erc20-benchmark, consider adding a similar # TODO: comment above the spamoor-benchmark job to make it clear it's intentionally not yet wired into publishing. This would help future contributors understand why the job exists but isn't part of publish-benchmarks.

  # TODO: wire up to publish results once additional tests are in place.
  spamoor-benchmark:

No issues found with the actual removal logic — the publish-benchmarks job correctly drops the needs dependency and the corresponding download/store steps. The if: always() pattern and Reset local gh-pages branch step are preserved correctly for the remaining two benchmark stores.

Overall: LGTM. Clean, intentional reduction in scope with no side effects.

@chatton chatton marked this pull request as ready for review March 5, 2026 09:14
@julienrbrt julienrbrt changed the title chore: remove spamoor results from benchmark results per PR ci: remove spamoor results from benchmark results per PR Mar 5, 2026
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.80%. Comparing base (5a07bc3) to head (a88badc).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3138      +/-   ##
==========================================
- Coverage   60.82%   60.80%   -0.03%     
==========================================
  Files         113      113              
  Lines       11695    11695              
==========================================
- Hits         7114     7111       -3     
- Misses       3777     3781       +4     
+ Partials      804      803       -1     
Flag Coverage Δ
combined 60.80% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chatton chatton enabled auto-merge March 5, 2026 09:22
@julienrbrt julienrbrt disabled auto-merge March 5, 2026 10:00
@julienrbrt julienrbrt merged commit f0eba0d into main Mar 5, 2026
39 of 40 checks passed
@julienrbrt julienrbrt deleted the cian/disable-spamoor-benchmarks-being-published branch March 5, 2026 10:00
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