Skip to content

Conversation

@wtommy932
Copy link
Contributor

No description provided.

@wtommy932 wtommy932 requested review from Pigeon0v0 and ruattd January 23, 2026 05:34
@pcl-ce-automation pcl-ce-automation bot added 🛠️ 等待审查 Pull Request 已完善,等待维护者或负责人进行代码审查 size: XXL PR 大小评估:巨型 labels Jan 23, 2026
Copy link
Contributor

@ruattd ruattd left a comment

Choose a reason for hiding this comment

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

从 VB 把史山 Copy-Paste 到 C# 没什么意义

我衷心建议你去读读别的优秀开源项目源码,自己写写实际项目沉淀一下,再考虑动 CE 的核心设施

以及,你起名字的能力确实不太够,要不借助一下 AI?

Comment on lines +21 to +25
/// <summary>
/// 下载更新文件
/// </summary>
/// <param name="outputPath">输出路径</param>
public Task<bool> DownloadAsync(string outputPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

更新源自己负责下载会导致很多重复的可复用内容


public string SourceName => name;

private static readonly string _TempPath = Path.Combine(FileService.TempPath, "Cache", "Update");
Copy link
Contributor

Choose a reason for hiding this comment

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

《TempPath》《Cache》《Update》

很难不让人怀疑你在把缓存目录当数据目录用

Copy link
Contributor

Choose a reason for hiding this comment

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

这个实现里我注意到了很多抛出的异常,你在上层有健全的异常处理来利用这个抛出异常的机制吗,还是仅仅把它直接丢给 Lifecycle 或 UI 层的保底处理?

这里抛出异常这种设计本身就不是很合理,况且一般使用这种设计传递信息都是会自定义异常的,然而你抛的都是stdlib 自带的异常

Comment on lines +66 to +78
_LogTrace("开始获取版本信息");
var channelName = Config.System.Update.UpdateChannel switch
{
0 => "sr",
1 => "fr",
_ => "sr"
} + (RuntimeInformation.ProcessArchitecture == Architecture.Arm64 ? "arm64" : "x64");

var assets = await _GetRemoteInfoByNameAsync<VersionAssetsDataModel>(
$"updates-{channelName}", "updates/").ConfigureAwait(false);
_LogTrace("版本信息获取完成");

ret = assets?.Assets.FirstOrDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

目前更新是打算改成逐层检查的逻辑的,这里仍然用了传统的逻辑,而且这段逻辑可以复用吧

Comment on lines +60 to +63
public Task<AnnouncementsListModel> GetAnnouncementAsync()
{
throw new NotSupportedException("MirrorChyan 更新源不支持公告功能");
}
Copy link
Contributor

@ruattd ruattd Jan 23, 2026

Choose a reason for hiding this comment

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

为什么不直接设计成返回 nullable 呢,这点 unsupported 的破事都要抛个异常,不就是把史山直接搬过来了

Comment on lines +123 to +125
_LogInfo("开始下载更新文件");
var manager = new DownloadManager(new FastMirrorSelector(NetworkService.GetClient()));
await manager.DownloadAsync(task, CancellationToken.None).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

虽然不知道这个下载器支不支持但是提个醒,这个下载源最好限制单线程

Comment on lines +244 to +248

/// <summary>
/// 已显示的启动器公告。
/// </summary>
[ConfigItem<string>("SystemSystemAnnouncement", "", ConfigSource.Local)] public partial string ShowedAnnouncement { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

直接用 List

Copy link
Contributor

Choose a reason for hiding this comment

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

你这个 RemoteInfo 干的事跟这个名字八竿子打不着


[LifecycleService(LifecycleState.BeforeLoading)]
public sealed class UpdateService : GeneralService
[LifecycleScope("update", "处理更新参数")]
Copy link
Contributor

Choose a reason for hiding this comment

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

真的只是“处理更新参数”吗

@wtommy932
Copy link
Contributor Author

唉...我应该再关PR再次重写

@wtommy932 wtommy932 closed this Jan 23, 2026
@pcl-ce-automation pcl-ce-automation bot removed 🛠️ 等待审查 Pull Request 已完善,等待维护者或负责人进行代码审查 size: XXL PR 大小评估:巨型 labels Jan 23, 2026
@wtommy932 wtommy932 deleted the feat/update branch January 23, 2026 07:29
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.

3 participants