Skip to content

Conversation

@electricface
Copy link
Member

@electricface electricface commented Jan 30, 2026

  • Changed processorChanged call to explicitly emit signal using
    Q_EMIT processorChanged
  • Added comments explaining that processor is not set in activate
    to avoid duplicate settings

fix(systeminfo): 修复 processorChanged 信号发射和 processor
设置位置

  • 将 processorChanged 调用改为显式发射信号 Q_EMIT
    processorChanged
  • 注释说明不在 activate 中设置 processor,避免重复设置

PMS: BUG-274991
Influence: 控制中心->系统->关于本机->处理器

Summary by Sourcery

Fix processor change handling in the system info plugin to ensure correct signal emission and avoid duplicate processor updates.

Bug Fixes:

  • Emit the processorChanged signal correctly when the processor value is updated.
  • Prevent duplicate processor value setting by relying on the constructor's updateFrequency() initialization instead of activating logic.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 30, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts how the processorChanged signal is emitted and clarifies where the processor field is set to avoid duplicate updates in the system info plugin.

Sequence diagram for updated processorChanged emission and processor setting flow

sequenceDiagram
    actor User
    participant SystemInfoPluginUI
    participant SystemInfoWork
    participant SystemInfoModel

    User ->> SystemInfoPluginUI: openSystemInfoAboutPage
    SystemInfoPluginUI ->> SystemInfoWork: createInstance
    activate SystemInfoWork
    SystemInfoWork ->> SystemInfoModel: constructor
    SystemInfoModel ->> SystemInfoModel: updateFrequency
    SystemInfoModel ->> SystemInfoModel: setProcessor(processor)
    SystemInfoModel ->> SystemInfoModel: Q_EMIT processorChanged(processor)
    SystemInfoModel -->> SystemInfoPluginUI: processorChanged(processor)
    deactivate SystemInfoWork

    User ->> SystemInfoPluginUI: refreshSystemInfo
    SystemInfoPluginUI ->> SystemInfoWork: activate
    SystemInfoWork ->> SystemInfoModel: setType(QSysInfo_WordSize)
    SystemInfoWork ->> SystemInfoModel: setKernel(kernelVersion)
    Note over SystemInfoWork,SystemInfoModel: processor is not set in activate to avoid duplicate processorChanged
    SystemInfoModel -->> SystemInfoPluginUI: existing processor value used for display
Loading

Class diagram for SystemInfoModel and SystemInfoWork processor handling changes

classDiagram
    class SystemInfoModel {
        - QString m_processor
        + SystemInfoModel()
        + void setProcessor(QString processor)
        + void setType(int wordSize)
        + void setKernel(QString kernelVersion)
        + void updateFrequency()
        + void processorChanged(QString processor)
    }

    class SystemInfoWork {
        - SystemInfoModel* m_model
        + SystemInfoWork()
        + void activate()
    }

    SystemInfoWork --> SystemInfoModel : uses
    SystemInfoModel ..> SystemInfoModel : Q_EMIT processorChanged
Loading

File-Level Changes

Change Details Files
Emit the processorChanged signal correctly from SystemInfoModel when the processor value is updated.
  • Replace direct processorChanged method call with explicit Q_EMIT processorChanged signal emission in the processor setter
  • Ensure the internal m_processor field is updated before emitting the signal
src/plugin-systeminfo/operation/systeminfomodel.cpp
Prevent processor from being set twice during activation and document the intended behavior.
  • Remove processor initialization in activate() that duplicated logic performed in the constructor via updateFrequency()
  • Add a comment explaining that processor should not be set in activate() to avoid duplicate settings
src/plugin-systeminfo/operation/systeminfowork.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Relying on processor initialization inside the constructor’s updateFrequency() is a bit implicit; consider either moving the processor setup into a clearly named initialization method or expanding the comment to briefly mention where processor is set to make the control flow easier to follow.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Relying on processor initialization inside the constructor’s updateFrequency() is a bit implicit; consider either moving the processor setup into a clearly named initialization method or expanding the comment to briefly mention where processor is set to make the control flow easier to follow.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

setting location

- Changed processorChanged call to explicitly emit signal using
Q_EMIT processorChanged
- Added comments explaining that processor is not set in activate
to avoid duplicate settings

---

fix(systeminfo): 修复 processorChanged 信号发射和 processor
设置位置

- 将 processorChanged 调用改为显式发射信号 Q_EMIT
processorChanged
- 注释说明不在 activate 中设置 processor,避免重复设置

Bug: https://pms.uniontech.com/bug-view-274991.html
PMS: BUG-274991
Influence: 控制中心->系统->关于本机->处理器
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@fly602
Copy link
Contributor

fly602 commented Feb 2, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 2, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 74ab408 into linuxdeepin:master Feb 2, 2026
15 of 18 checks passed
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要涉及Qt信号/槽机制的规范使用以及业务逻辑的调整。以下是对代码变更的审查意见:

1. 语法逻辑

  • 变更点 1 (systeminfomodel.cpp):

    • 修改前: processorChanged(processor);
    • 修改后: Q_EMIT processorChanged(processor);
    • 审查意见: 改进正确。在 Qt 中,信号(Signal)本质上是 protected 作用域的函数,直接调用虽然能编译通过,但不符合 Qt 的元对象系统(Meta-Object System)的最佳实践。使用 Q_EMIT 宏(或在旧版本中使用 emit)可以明确表明这是一个信号的发送操作,提高代码的可读性,并且有助于一些静态分析工具识别信号槽连接。
  • 变更点 2 (systeminfowork.cpp):

    • 修改前: m_model->setProcessor(DSysInfo::cpuModelName());
    • 修改后: // 注意:不在此处设置 processor,因为构造函数中的 updateFrequency() 已经设置了 processor
    • 审查意见: 逻辑需确认。这里删除了一行设置处理器信息的代码,并添加了注释说明原因。从逻辑上看,这是为了避免重复设置或覆盖之前的数据。潜在风险在于:
      1. 时序问题:如果 activate() 可能在对象生命周期内被多次调用,而 updateFrequency() 仅在构造函数中调用一次,那么 activate() 中移除该调用是合理的,前提是 updateFrequency() 中的数据源是最终且准确的。
      2. 数据一致性:需要确认 updateFrequency() 中获取 CPU 信息的方式与 DSysInfo::cpuModelName() 是否完全一致。如果 updateFrequency() 使用了其他接口(例如读取 /proc/cpuinfo 的不同字段),可能会导致显示的 CPU 型号不一致。

2. 代码质量

  • 变更点 1: 使用 Q_EMIT 宏提升了代码的规范性和可读性,符合 Qt 编码规范。
  • 变更点 2: 添加的注释解释了代码修改的原因,这对维护代码非常有帮助。但是,建议在注释中进一步说明 updateFrequency() 中的具体逻辑,或者引用相关的 Issue/Task 编号,以便后续维护者了解背景。

3. 代码性能

  • 变更点 1: Q_EMIT 是宏展开,在编译期处理,对运行时性能无影响
  • 变更点 2: 移除了一次 DSysInfo::cpuModelName() 的调用和一次 setProcessor 的调用。如果 DSysInfo::cpuModelName() 内部涉及读取系统文件或执行系统命令,那么这一改动减少了系统调用,提升了性能

4. 代码安全

  • 变更点 1: 无安全性影响。
  • 变更点 2: 无直接的安全性影响。但需注意上述提到的数据一致性问题,如果 CPU 信息获取逻辑不一致,可能导致显示错误信息,属于逻辑层面的稳定性问题。

总结与改进建议

这段代码修改整体是积极的,提升了代码规范性和性能。针对 systeminfowork.cpp 中的逻辑修改,建议进一步确认以下几点:

  1. 验证数据源一致性:请务必检查 SystemInfoWork 构造函数中调用的 updateFrequency() 方法,确认其获取 CPU 名称的逻辑与被移除的 DSysInfo::cpuModelName() 返回值完全一致。如果不一致,需要评估哪种方式更准确。
  2. 完善注释:建议将注释写得更加具体,例如:
    // 注意:不在此处设置 processor。
    // 原因:构造函数中的 updateFrequency() 已通过读取 /proc/cpuinfo 设置了 processor,
    // 且 DSysInfo::cpuModelName() 返回的信息不如前者详细/准确。
  3. 考虑信号触发:如果在 activate() 中移除了 setProcessor,需确认 UI 层是否依赖 processorChanged 信号来刷新显示。如果 updateFrequency 在构造函数中执行,且 UI 绑定发生在构造之后,通常没有问题;但如果 activate 是用于刷新数据的入口,移除此调用可能导致 CPU 信息在某些场景下不更新。

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