Skip to content

ci(audio): clarify macOS prebuild artifact suffix - #6275

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
barry166:barry166/audio-artifact-suffix-5649
Jul 4, 2026
Merged

ci(audio): clarify macOS prebuild artifact suffix#6275
wenshao merged 1 commit into
QwenLM:mainfrom
barry166:barry166/audio-artifact-suffix-5649

Conversation

@barry166

@barry166 barry166 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What this PR does

  • Clarifies the macOS audio prebuild artifact name by using an explicit arm64+x64 suffix.
  • Keeps the existing artifact naming for the other matrix entries unchanged.

AI-assisted: yes
Human reviewed: yes

Why it's needed

The macOS audio prebuild job now produces a universal archive from the macos-14 runner, but the upload artifact name still advertised only arm64. The explicit suffix makes the artifact name match the contents without changing the collect job's prebuilds-* download pattern.

Reviewer Test Plan

How to verify

  • Review .github/workflows/audio-capture-prebuilds.yml and confirm only the macOS matrix entry gets artifact_suffix: 'arm64+x64'.
  • Confirm the upload artifact expression falls back to matrix.arch for other platforms.
  • Confirm the collect job still downloads prebuilds-* artifacts unchanged.

Evidence (Before & After)

  • Before: macOS upload artifact name used prebuilds-macos-14-arm64 even though the job packages both arm64 and x64 slices.
  • After: macOS upload artifact name uses prebuilds-macos-14-arm64+x64; all other artifact names are unchanged.

Tested on

  • actionlint v1.7.7 .github/workflows/audio-capture-prebuilds.yml
  • Node YAML parse for .github/workflows/audio-capture-prebuilds.yml
  • git diff --check

Risk & Scope

Low risk. This only changes the CI artifact display/upload name for the macOS prebuild matrix entry and leaves artifact collection patterns unchanged.

Linked Issues

Fixes #5649

The macOS audio prebuild job now packages both arm64 and x64 slices from the macos-14 runner, but the artifact name still advertised only arm64. Add a matrix suffix so that upload-artifact names the macOS bundle accurately while leaving other platform names unchanged.

Constraint: Existing collect job downloads prebuilds-* and should not need a pattern change
Confidence: high
Scope-risk: narrow
Tested: actionlint v1.7.7 .github/workflows/audio-capture-prebuilds.yml
Tested: node YAML parse for .github/workflows/audio-capture-prebuilds.yml
Tested: git diff --check

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @barry166, thanks for the contribution!

The PR body doesn't follow our pull request template. The required headings are missing:

  • ## What this PR does
  • ## Why it's needed
  • ## Reviewer Test Plan (with "How to verify", "Evidence (Before & After)", and "Tested on" sub-sections)
  • ## Risk & Scope
  • ## Linked Issues

The current headings (Summary, Testing, Review notes) don't match. Could you restructure the PR body to use the template? It helps reviewers understand the change and verify it works — especially the "Reviewer Test Plan" section, even for CI-only changes (e.g., paste the actionlint output and confirm collect still merges correctly).

中文说明

Hi @barry166,感谢贡献!

PR 正文没有按照我们的 PR 模板 填写。缺少以下必填标题:

  • ## What this PR does
  • ## Why it's needed
  • ## Reviewer Test Plan(包含 "How to verify"、"Evidence (Before & After)" 和 "Tested on" 子部分)
  • ## Risk & Scope
  • ## Linked Issues

当前的标题(SummaryTestingReview notes)与模板不匹配。请按照模板重新组织 PR 正文。这有助于 reviewer 理解变更并验证效果——即使是纯 CI 改动也需要 "Reviewer Test Plan" 部分(例如贴上 actionlint 的输出,并确认 collect 仍能正确合并)。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM!

— GPT-5 via Qwen Code /review

@wenshao

wenshao commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR @barry166! (Re-running triage — the PR body has been updated since the last run.)

Template looks good now ✓ — all required sections are present.

Problem: real and well-documented. Issue #5649 identified this naming inconsistency as a follow-up from PR #5643 — the macOS artifact says arm64 but contains both slices. Not theoretical.

Direction: straightforward CI naming fix. Clearly within scope — artifact names should accurately describe their contents.

Approach: minimal (2+/1-), exactly matches the suggested fix from #5649. Focused, no scope creep.

Moving on to code review. 🔍

中文说明

感谢 PR @barry166!(重新运行 triage——PR 正文已更新。)

模板现在完整 ✓

问题:真实存在,在 issue #5649 中作为 PR #5643 的 follow-up 被记录。macOS artifact 名称标注为 arm64 但实际包含两种架构。非理论性问题。

方向:简单的 CI 命名修复,明确在项目范围内。

方案:最小改动(2+/1-),与 #5649 建议的修复方式完全一致。范围聚焦,无额外改动。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The diff is clean and correct. Two changes, both necessary:

  1. Adds artifact_suffix: 'arm64+x64' to the macOS matrix entry — scoped to only the entry that produces both slices.
  2. Changes the upload name from matrix.arch to matrix.artifact_suffix || matrix.arch — uses the override when defined, falls back to matrix.arch for all other entries.

The GitHub Actions expression ${{ matrix.artifact_suffix || matrix.arch }} is valid — || is the standard fallback operator in expression syntax. The collect job downloads with pattern: 'prebuilds-*' + merge-multiple: true, so the renamed artifact (prebuilds-macos-14-arm64+x64) is still matched.

No correctness bugs, no security concerns, no convention violations. This is a textbook minimal CI fix.

Real-Scenario Testing

N/A — this is a CI workflow change with no user-visible behavior. The audio-capture-prebuilds workflow is workflow_call only (not triggered by push/PR), so it won't run as part of this PR's CI checks. Verification is limited to:

  • YAML validity: the workflow parses correctly (author confirmed via actionlint and Node YAML parse).
  • Expression correctness: ${{ matrix.artifact_suffix || matrix.arch }} is standard GitHub Actions syntax.
  • Pattern compatibility: prebuilds-* glob in the collect job matches prebuilds-macos-14-arm64+x64.

All 9 non-skipped CI checks on this PR are green.

中文说明

代码审查

diff 干净且正确。两处改动都是必要的:

  1. 为 macOS matrix 条目添加 artifact_suffix: 'arm64+x64'——仅作用于产生两种架构的条目。
  2. 将上传名称从 matrix.arch 改为 matrix.artifact_suffix || matrix.arch——有定义时使用覆盖值,其他条目回退到 matrix.arch

GitHub Actions 表达式 ${{ matrix.artifact_suffix || matrix.arch }} 语法正确。collect job 使用 pattern: 'prebuilds-*' + merge-multiple: true 下载,重命名后的 artifact(prebuilds-macos-14-arm64+x64)仍会被匹配。

无正确性 bug、无安全隐患、无规范违反。

真实场景测试

不适用——这是纯 CI workflow 改动,无用户可见行为。audio-capture-prebuilds workflow 仅通过 workflow_call 触发,不会作为此 PR 的 CI 检查运行。

所有 9 项非跳过的 CI 检查均为绿色。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Stepping back: this is about as clean as a CI naming fix gets. The problem is real (documented in #5649 as a follow-up to #5643), the solution is the exact 3-line change the issue suggested, and my independent proposal was identical. The diff does one thing, does it minimally, and the collect job is unaffected.

The previous triage run's CHANGES_REQUESTED was about template compliance — the author has since updated the PR body with all required sections. That concern is resolved.

A maintainer has already approved. All CI checks are green. No reservations.

Approving. ✅

中文说明

总结:这是一个非常干净的 CI 命名修复。问题真实存在(#5649 中作为 #5643 的 follow-up 记录),方案就是 issue 中建议的 3 行改动,我的独立提案与之完全一致。diff 只做一件事,改动最小,collect job 不受影响。

上一次 triage 的 CHANGES_REQUESTED 是关于模板合规——作者已更新 PR 正文,该问题已解决。

maintainer 已批准。所有 CI 检查为绿色。无顾虑。

批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 4, 2026
Merged via the queue into QwenLM:main with commit 18e2eed Jul 4, 2026
53 checks passed
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.

Follow up: clarify macOS audio prebuild artifact naming

3 participants