-
Notifications
You must be signed in to change notification settings - Fork 21
ci: Add Winget publishing workflow and build support #470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - 我发现了 1 个问题,并给出了一些高层次的反馈:
- 在
xtask/src/build.rs中,opts.rename.unwrap_or(exe)无法通过编译,因为rename是Option<String>而exe是&str;可以改为类似opts.rename.unwrap_or_else(|| exe.to_string())的写法。 - 在
create_archive中,if target.contains("-windows-msvc")这个分支也会匹配 winget 目标(例如-windows-msvc-winget),因此 winget 专用的bin_name永远不会被使用;应当先检查-windows-msvc-winget这种更具体的情况,再处理通用的 Windows 情况。
面向 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- In `xtask/src/build.rs`, `opts.rename.unwrap_or(exe)` will not compile because `rename` is an `Option<String>` and `exe` is a `&str`; use something like `opts.rename.unwrap_or_else(|| exe.to_string())` instead.
- In `create_archive`, the `if target.contains("-windows-msvc")` branch will also match winget targets (e.g. `-windows-msvc-winget`), so the winget-specific `bin_name` is never used; check for the `-windows-msvc-winget` case before the generic Windows case.
## Individual Comments
### Comment 1
<location> `xtask/src/release/package.rs:159-161` </location>
<code_context>
// Use tar.gz for Unix-like systems (Linux, macOS) and zip for Windows
let (format, bin_name) = if target.contains("-windows-msvc") {
(ArchiveFormat::Zip, "maa.exe")
+ } else if target.contains("-windows-msvc-winget") {
+ (ArchiveFormat::Zip, "maa-cli.exe")
} else if target.contains("-linux-") || target.contains("-apple-darwin") {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `-windows-msvc-winget` branch is unreachable due to the broader `-windows-msvc` match above.
Because `target.contains("-windows-msvc")` matches `...-windows-msvc-winget`, the `else if target.contains("-windows-msvc-winget")` branch is never taken, so winget builds still get `maa.exe` instead of `maa-cli.exe`.
Swap the order so the more specific pattern is checked first:
```rust
let (format, bin_name) = if target.contains("-windows-msvc-winget") {
(ArchiveFormat::Zip, "maa-cli.exe")
} else if target.contains("-windows-msvc") {
(ArchiveFormat::Zip, "maa.exe")
} else if target.contains("-linux-") || target.contains("-apple-darwin") {
// ...
}
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
xtask/src/build.rs,opts.rename.unwrap_or(exe)will not compile becauserenameis anOption<String>andexeis a&str; use something likeopts.rename.unwrap_or_else(|| exe.to_string())instead. - In
create_archive, theif target.contains("-windows-msvc")branch will also match winget targets (e.g.-windows-msvc-winget), so the winget-specificbin_nameis never used; check for the-windows-msvc-wingetcase before the generic Windows case.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `xtask/src/build.rs`, `opts.rename.unwrap_or(exe)` will not compile because `rename` is an `Option<String>` and `exe` is a `&str`; use something like `opts.rename.unwrap_or_else(|| exe.to_string())` instead.
- In `create_archive`, the `if target.contains("-windows-msvc")` branch will also match winget targets (e.g. `-windows-msvc-winget`), so the winget-specific `bin_name` is never used; check for the `-windows-msvc-winget` case before the generic Windows case.
## Individual Comments
### Comment 1
<location> `xtask/src/release/package.rs:159-161` </location>
<code_context>
// Use tar.gz for Unix-like systems (Linux, macOS) and zip for Windows
let (format, bin_name) = if target.contains("-windows-msvc") {
(ArchiveFormat::Zip, "maa.exe")
+ } else if target.contains("-windows-msvc-winget") {
+ (ArchiveFormat::Zip, "maa-cli.exe")
} else if target.contains("-linux-") || target.contains("-apple-darwin") {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `-windows-msvc-winget` branch is unreachable due to the broader `-windows-msvc` match above.
Because `target.contains("-windows-msvc")` matches `...-windows-msvc-winget`, the `else if target.contains("-windows-msvc-winget")` branch is never taken, so winget builds still get `maa.exe` instead of `maa-cli.exe`.
Swap the order so the more specific pattern is checked first:
```rust
let (format, bin_name) = if target.contains("-windows-msvc-winget") {
(ArchiveFormat::Zip, "maa-cli.exe")
} else if target.contains("-windows-msvc") {
(ArchiveFormat::Zip, "maa.exe")
} else if target.contains("-linux-") || target.contains("-apple-darwin") {
// ...
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #470 +/- ##
=======================================
Coverage 69.34% 69.34%
=======================================
Files 56 56
Lines 5598 5598
Branches 5598 5598
=======================================
Hits 3882 3882
Misses 1425 1425
Partials 291 291 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #428.
Summary by Sourcery
为通过 Winget 分发的 Windows CLI 工件添加构建和发布的 CI 支持。
新功能:
改进:
CI:
Co-authored-by: JasonHuang79 63983700+JasonHuang79@users.noreply.github.com