ci: publish benchmarks in repository Pages - #2268
Conversation
There was a problem hiding this comment.
Review summary
This PR switches baseline benchmark publishing to the same-repo GitHub Pages model: it points site-path at benchmark/baseline and drops the external-data-repo wiring (data_repository, data_dispatch_event, secrets.data_token) from benchmark-publish.yml.
The workflow changes themselves are correct and safe. I verified against the reusable workflow xgo-dev/setup-benchmark-go-action/.github/workflows/publish.yml@v1:
data_repositorydefaults to""→ falls back to the source repo (xgo-dev/llgo), matching the intent.data_tokenisrequired: false; same-repo commits use the built-in token, so removing the secret is fine.data_dispatch_eventdefaults to""(no dispatch), appropriate for same-repo publishing.run_id(the only required input) andconfig_pathare retained.- The new
site-path: benchmark/baselinematches the source dir./benchmark/baselineused in.github/workflows/benchmark.yml.
The findings below are all in benchmark/baseline/README.md, which is not part of this diff and so cannot be commented inline.
Findings
1. Stale documentation — benchmark/baseline/README.md:25-28
The README still says the publisher "commits ... to the pages branch of the configured data repository. Every LLGo repository defaults to <owner>/llgo-benchmark-data." After this PR, publishing defaults to the source repo (xgo-dev/llgo) Pages, not a separate <owner>/llgo-benchmark-data repository. This description is now inaccurate.
2. Stale series paths — benchmark/baseline/README.md:30-34
The documented series paths use the old site-path prefix:
llgo/baseline/series/main/main
llgo/baseline/series/branch/<safe branch identifier>
llgo/baseline/series/pull/<number>
Since site-path is now benchmark/baseline, these should read benchmark/baseline/series/....
3. Stale token statement — benchmark/baseline/README.md:36-37
"pull request jobs never receive the benchmark repository token" describes the removed external-data-repo token mechanism, which no longer exists in this flow. Consider rewording to reflect same-repo publishing.
Non-blocking notes
- Security (low): With
data_tokenremoved, same-repo commits now use the defaultGITHUB_TOKEN, which this workflow grantscontents: write,issues: write,pull-requests: write. Consider pinning the reusable workflow to a commit SHA instead of the mutable@v1tag, and trimming permissions to the minimum the publish flow needs (e.g. dropissues: writeif only PR comments are used). Not exploitable on its own; theworkflow_run+ fork-PR data flow mitigations (trusted config read from default branch, PR-number namespacing, head-SHA re-check) live in the callee and are unchanged by this diff. - Behavior: Removing
data_dispatch_eventmeans norepository_dispatchis sent after publish. This is expected for same-repo publishing, but confirm no downstream deploy relied on that event.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
No main baseline exists yet; all metrics are marked Warning
|
Summary
pagesbranch without a separate data repository or dispatch tokenbenchmark/baselineto avoid collisions with other Pages contentThis ports the two commits from cpunion#141. The cpunion source PR is merged and its source branch has been deleted; this PR is the xgo-dev counterpart.
Validation
.github/workflows/benchmark-publish.ymland.github/llgo-benchmark.ymlgit diff --checkpasses