Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Nov 10, 2025

Using linuxdeepin/dde-control-center@f86903a as reference.

Summary by Sourcery

Add installation of debug and log configuration files, extend license annotations to JSON files, and standardize logging category identifiers across the shell components

Enhancements:

  • Install debug and log JSON configuration files via CMake
  • Standardize QLoggingCategory names to use fully-qualified "org.deepin.dde.shell..." prefixes throughout the codebase

Build:

  • Add CMake install rules for misc/deepin-debug-config and misc/deepin-log-config JSON files

Documentation:

  • Extend REUSE.toml license annotations to include JSON configuration files

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 10, 2025

Reviewer's Guide

This PR enhances the debugging settings by installing new JSON-based debug and log configurations via CMake, standardizes all QLoggingCategory identifiers to the "org.deepin.dde.shell" namespace across modules, and updates REUSE.toml to include JSON files in its compliance patterns.

Entity relationship diagram for REUSE.toml compliance patterns update

erDiagram
    REUSE_TOML {
        path string
        precedence string
        SPDX-FileCopyrightText string
        SPDX-LicenseIdentifier string
    }
    REUSE_TOML ||--o| JSON_FILE : includes
    REUSE_TOML ||--o| SERVICE_FILE : includes
    JSON_FILE {
        file string
    }
    SERVICE_FILE {
        file string
    }
Loading

Class diagram for QLoggingCategory usage after namespace standardization

classDiagram
    class AppGroup {
        +QLoggingCategory appGroupLog : "org.deepin.dde.shell.dde-apps.appgroup"
    }
    class AmAppItemModel {
        +QLoggingCategory appsLog : "org.deepin.dde.shell.dde-apps.amappitemmodel"
    }
    class DockPanel {
        +QLoggingCategory dockLog : "org.deepin.dde.shell.dock"
    }
    class DockSettings {
        +QLoggingCategory dockSettingsLog : "org.deepin.dde.shell.dock.docksettings"
    }
    class ShowDesktop {
        +QLoggingCategory showDesktop : "org.deepin.dde.shell.dock.showdesktop"
    }
    class AppItem {
        +QLoggingCategory appitemLog : "org.deepin.dde.shell.dock.taskmanger.appitem"
    }
    class DesktopFileAbstractParser {
        +QLoggingCategory abstractdesktopfileLog : "org.deepin.dde.shell.dock.abstractdesktopfile"
    }
    class DockGlobalElementModel {
        +QLoggingCategory dockGlobalElementModelLog : "org.deepin.dde.shell.dock.taskmanager.dockglobalelementmodel"
    }
    class TaskManager {
        +QLoggingCategory taskManagerLog : "org.deepin.dde.shell.dock.taskmanager"
    }
    class X11Window {
        +QLoggingCategory x11windowLog : "org.deepin.dde.shell.dock.taskmanager.x11window"
    }
    class X11WindowMonitor {
        +QLoggingCategory x11Log : "org.deepin.dde.shell.dock.taskmanager.x11windowmonitor"
    }
    class DockX11Helper {
        +QLoggingCategory dockX11Log : "org.deepin.dde.shell.dock.x11"
    }
    class NotifyDBAccessor {
        +QLoggingCategory notifyDBLog : "org.deepin.dde.shell.notification.db"
    }
    class NotifyEntity {
        +QLoggingCategory notifyLog : "org.deepin.dde.shell.notification"
    }
    class NotificationContainment {
        +QLoggingCategory notifyLog : "org.deepin.dde.shell.notification"
    }
    class OsdPanel {
        +QLoggingCategory osdLog : "org.deepin.dde.shell.osd"
    }
    class DisplayModeApplet {
        +QLoggingCategory osdDPLog : "org.deepin.dde.shell.osd.display"
    }
    class KbLayoutApplet {
        +QLoggingCategory osdKBLog : "org.deepin.dde.shell.osd.kblayout"
    }
    class DSQuickDrag {
        +QLoggingCategory dsDragLog : "org.deepin.dde.shell.drag"
    }
    class DSPluginMetaData {
        +QLoggingCategory dsLog : "org.deepin.dde.shell"
    }
    class DSLoader {
        +QLoggingCategory dsLoaderLog : "org.deepin.dde.shell.loader"
    }
    class LayerShellWindow {
        +QLoggingCategory layershellwindow : "org.deepin.dde.shell.layershell.window"
    }
    class LayerShellSurface {
        +QLoggingCategory layershellsurface : "org.deepin.dde.shell.layershell.surface"
    }
    class LayerShellEmulation {
        +QLoggingCategory layershell : "org.deepin.dde.shell.layershell"
    }
Loading

File-Level Changes

Change Details Files
Install deepin-debug-config and deepin-log-config JSON files through CMake
  • Added install directive for deepin-debug-config JSON
  • Added install directive for deepin-log-config JSON
shell/CMakeLists.txt
misc/deepin-debug-config/org.deepin.dde.shell.json
misc/deepin-log-config/org.deepin.dde.shell.json
Normalize QLoggingCategory identifiers to the new namespace
  • Prefixed all QLoggingCategory definitions with "org.deepin.dde.shell"
  • Replaced legacy category strings in various modules
panels/dock/taskmanager/desktopfileabstractparser.cpp
applets/dde-apps/amappitemmodel.cpp
applets/dde-apps/appgroup.cpp
frame/layershell/dlayershellwindow.cpp
frame/layershell/qwaylandlayershellsurface.cpp
frame/layershell/x11dlayershellemulation.cpp
frame/pluginmetadata.cpp
frame/quick/dsquickdrag.cpp
panels/dock/dockpanel.cpp
panels/dock/docksettings.cpp
panels/dock/showdesktop/showdesktop.cpp
panels/dock/taskmanager/appitem.cpp
panels/dock/taskmanager/desktopfileamparser.cpp
panels/dock/taskmanager/dockglobalelementmodel.cpp
panels/dock/taskmanager/taskmanager.cpp
panels/dock/taskmanager/treelandwindow.cpp
panels/dock/taskmanager/x11preview.cpp
panels/dock/taskmanager/x11utils.cpp
panels/dock/taskmanager/x11window.cpp
panels/dock/taskmanager/x11windowmonitor.cpp
panels/dock/x11dockhelper.cpp
panels/notification/common/dbaccessor.cpp
panels/notification/common/notifyentity.cpp
panels/notification/notificationcontainment.cpp
panels/notification/osd/displaymode/displaymodeapplet.cpp
panels/notification/osd/kblayout/kblayoutapplet.cpp
panels/notification/osd/osdpanel.cpp
shell/appletloader.cpp
Extend REUSE.toml patterns to include JSON files
  • Modified annotation path to cover .json files in misc
REUSE.toml

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 there - I've reviewed your changes - here's some feedback:

  • In CMakeLists.txt you’re using DCC_PROJECT_ROOT_DIR to locate shell JSON files—consider defining and using a project‐specific variable (or CMAKE_SOURCE_DIR) to avoid coupling with the control‐center project.
  • There are typos in some logging category identifiers (for example "taskmanger" instead of "taskmanager"); please correct those to keep category names consistent.
  • The REUSE.toml update switches to an array for the path field—verify that the REUSE toolchain supports this syntax to prevent license validation breaks.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In CMakeLists.txt you’re using DCC_PROJECT_ROOT_DIR to locate shell JSON files—consider defining and using a project‐specific variable (or CMAKE_SOURCE_DIR) to avoid coupling with the control‐center project.
- There are typos in some logging category identifiers (for example "taskmanger" instead of "taskmanager"); please correct those to keep category names consistent.
- The REUSE.toml update switches to an array for the path field—verify that the REUSE toolchain supports this syntax to prevent license validation breaks.

## Individual Comments

### Comment 1
<location> `panels/dock/taskmanager/appitem.cpp:19` </location>
<code_context>
 #include <QLoggingCategory>

-Q_LOGGING_CATEGORY(appitemLog, "dde.shell.dock.taskmanger.appitem")
+Q_LOGGING_CATEGORY(appitemLog, "org.deepin.dde.shell.dock.taskmanger.appitem")

 namespace dock {
</code_context>

<issue_to_address>
**issue (typo):** Typo in logging category: 'taskmanger' should be 'taskmanager'.

This typo may cause issues when filtering logs by category.

```suggestion
Q_LOGGING_CATEGORY(appitemLog, "org.deepin.dde.shell.dock.taskmanager.appitem")
```
</issue_to_address>

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.

Using linuxdeepin/dde-control-center f86903a87b52048189d0d869c1828fc13da28c03
as reference.

Log:
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码修改进行审查分析:

  1. 语法逻辑:
  • 代码修改主要是将日志分类名称从简单的 "dde.shell.xxx" 格式改为更规范的 "org.deepin.dde.shell.xxx" 格式
  • 修改使用了统一的命名空间前缀,符合 Qt 日志系统的最佳实践
  • 新增的 JSON 配置文件格式正确,结构清晰
  1. 代码质量:
  • 修改提高了日志分类的可读性和规范性
  • 统一了命名风格,有利于日志管理和过滤
  • 新增的配置文件提供了完整的日志配置信息,便于调试和问题排查
  1. 代码性能:
  • 这些修改主要是配置和命名相关的更改,不会对运行时性能产生影响
  • 日志系统本身的开销保持不变
  1. 代码安全:
  • 修改不涉及安全相关的内容
  • 日志分类名称的规范化有助于更好地管理和保护日志信息

改进建议:

  1. 建议在代码中添加统一的日志分类命名规范文档,方便后续开发人员遵循
  2. 可以考虑创建一个统一的宏定义来管理日志分类前缀,避免硬编码
  3. 建议在代码审查中加入对日志分类命名规范的检查
  4. JSON 配置文件中可以考虑添加版本号和更新日期信息,便于追踪配置变更

总体来说,这次修改是积极的,提高了代码的可维护性和规范性,没有发现明显的安全隐患或性能问题。

"submodules": [
{
"name": "org.deepin.dde.shell",
"exec": "org.deepin.dde.shell.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是给到了哪里?

Copy link
Contributor

Choose a reason for hiding this comment

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

好像要给 deepin-debug-config 加上这个玩意

Copy link
Member Author

Choose a reason for hiding this comment

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

对,这个就是我不清楚的地方。我看别的项目(比如控制中心)也没放这个 sh 文件,deepin-debug-config 好像是另外的仓库。

Copy link
Contributor

Choose a reason for hiding this comment

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

deepin-debug-config 好像就是要给这个仓库提供相应的sh进去, 我看到他存在控制中心的.sh

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, caixr23

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

@BLumia BLumia merged commit cad809b into linuxdeepin:master Nov 11, 2025
8 of 11 checks passed
@BLumia BLumia deleted the log branch November 11, 2025 02:09
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.

4 participants