Fix/ci riscv toolchain sudo

[jizhenjun]

No description provided.

[github-actions]

🤖 AI Code Review

共审查 2 个变更文件

📁 .github/workflows/ci.yml

🟡 Cache missing_tools result — Called twice: once in if [ -z ... ], again in echo "Missing tools: ...". Each call re-runs the loop.

missing="$(missing_tools)"
if [ -z "$missing" ]; then
  echo "RISC-V execution tools already available."
else
  echo "Missing tools: $missing"

🟡 Consolidate ::error:: annotations — Four separate ::error:: lines create four red annotations in the GitHub UI. One summary + regular echoes for details:

echo "::error::Cannot install RISC-V toolchain: runner is not root and has no passwordless sudo."
echo "Tests need clang, ld.lld, qemu-riscv32. Either preinstall on the runner:"
echo "  apt-get install --no-install-recommends clang lld qemu-user"
echo "or grant the runner user passwordless sudo for apt-get."

💭 Commented-out schedule is misleading — The comment says it "will stay red until the self-hosted runner is provisioned." A weekly job that is intentionally red adds noise without adding signal. Either link a tracking issue or drop it.

💭 Comment volume — ~40 lines of comments across the diff. The context is genuinely useful, but the file is now hard to scan for the actual runs-on expression and toolchain logic. Consider trimming to 2–3 comments per section and moving the full rationale to a design doc or PR description.

---

📁 .github/workflows/topic06-benchmark.yml

Review

🟡 Consider expanding branch filter if release branches exist — Restricting push to [main] means pushing to a release/* branch will no longer trigger this benchmark. If you have any non-main branches where benchmarks should run on push (e.g., release cuts), add them to the list. If main is the only one that matters, the change is correct.

💭 Comment could be shorter — The 4-line comment is helpful, but something like this conveys the same info more compactly:

  # Restrict push to main to avoid duplicate runs with pull_request (same pattern as ci.yml)

The core change itself is clean and correct — this is the standard fix for the duplicate-trigger issue on PR branches. 👍

---