Skip to content

Conversation

@zhaohuiw42
Copy link
Contributor

Move osTreeFinalize into the first-check success path so finalize happens before a potential reboot prior to login.

Task: https://pms.uniontech.com/task-view-385219.html

Move osTreeFinalize into the first-check success path so finalize
happens before a potential reboot prior to login.

Task: https://pms.uniontech.com/task-view-385219.html
@github-actions
Copy link

github-actions bot commented Dec 30, 2025

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

You can retrigger this bot by commenting recheck in this Pull Request

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码的变更进行审查:

  1. 代码逻辑分析:
    这段代码主要是在处理系统升级后的检查逻辑。变更主要涉及了第二次检查(secondCheck)的处理顺序,将osTreeFinalize()的调用从secondCheck分支移到了之前的位置。

  2. 改进建议:

a) 代码结构和可读性:

  • 注释的位置不太合适,建议将注释放在对应代码块的上方
  • 注释内容中混用了中英文,建议统一使用中文或英文
  • 注释中的"ps:"前缀是不必要的,应该移除

b) 代码逻辑:

  • 这个变更看起来是为了在第一次检查完成后就执行osTreeFinalize(),这可能是为了确保系统状态的一致性
  • 但是这样的改动可能会影响原有的错误处理流程,建议添加更详细的日志记录

c) 代码安全性:

  • 建议在osTreeFinalize()调用前后添加更详细的日志,方便追踪问题
  • 可以考虑添加状态检查,确保系统处于合适的状态才执行finalize操作
  1. 建议的改进版本:
case firstCheck:
    if err := m.immutableManager.osTreeFinalize(); err != nil {
        logger.Warningf("Failed to finalize os tree after first check: %v", err)
    }
case secondCheck:
    // 登录后检查无异常,上报更新成功,更新baseline信息,还原grub配置
    if err := m.delRebootCheckOption(secondCheck); err != nil {
        logger.Warningf("Failed to delete reboot check option: %v", err)
    }
  1. 其他建议:
  • 考虑为这些检查操作添加更详细的文档说明
  • 可以考虑将这些检查逻辑抽取为独立的方法,提高代码的可维护性
  • 建议添加更多的错误处理和恢复机制

这个变更的意图是合理的,但代码实现上还可以进一步优化,特别是在错误处理和日志记录方面。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zhaohuiw42 zhaohuiw42 merged commit 7c0510e into linuxdeepin:master Dec 30, 2025
14 of 16 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.

3 participants