Skip to content

Revert "refactor: move dockiteminfo to shared frame library"#1557

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:revert-dockinfo
Apr 16, 2026
Merged

Revert "refactor: move dockiteminfo to shared frame library"#1557
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:revert-dockinfo

Conversation

@wjyrich

@wjyrich wjyrich commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

This reverts commit 154a3a2.

Summary by Sourcery

Revert the migration of dockiteminfo into the shared frame library by relocating it back into the dock panel modules and updating build configuration and includes accordingly.

Enhancements:

  • Restore dockiteminfo as a dock-specific component rather than a shared frame-level header/source.

Build:

  • Update dock, appruntimeitem, multitaskview, and tray CMake targets to compile dockiteminfo from the panels/dock directory instead of the shared frame library.
  • Remove dockiteminfo sources and headers from the dde-shell-frame build configuration.

Chores:

  • Normalize SPDX copyright year ranges in affected dock and tray source and CMake files.

@sourcery-ai

sourcery-ai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR reverts the prior refactor that moved dockiteminfo into the shared frame library by moving the implementation back under panels/dock and re‑wiring all CMake targets and includes accordingly, along with narrowing several SPDX copyright year ranges.

File-Level Changes

Change Details Files
Move dockiteminfo implementation back from frame library into the dock panel module.
  • Relocate dockiteminfo.cpp and dockiteminfo.h from the frame directory into panels/dock.
  • Update CMakeLists in panels/dock to build dockiteminfo directly in the dock target instead of via the frame library.
  • Remove dockiteminfo sources and headers from the dde-shell-frame target in frame/CMakeLists.txt and its PUBLIC_HEADERS/PRIVATE_HEADERS lists.
panels/dock/CMakeLists.txt
frame/CMakeLists.txt
panels/dock/dockiteminfo.cpp
panels/dock/dockiteminfo.h
Wire dockiteminfo into dock subcomponents that rely on it after relocation.
  • Add dockiteminfo.cpp/.h from panels/dock to the appruntimeitem shared library sources using absolute paths from CMAKE_SOURCE_DIR.
  • Add ../dockiteminfo.cpp/.h as sources for the dock-multitaskview and trayitem shared libraries in their respective CMakeLists.
  • Adjust multitaskview.h to include dockiteminfo via a relative path one directory up ("../dockiteminfo.h") instead of the previous frame-based include.
panels/dock/appruntimeitem/CMakeLists.txt
panels/dock/multitaskview/CMakeLists.txt
panels/dock/multitaskview/multitaskview.h
panels/dock/tray/CMakeLists.txt
Normalize SPDX copyright headers to single-year values in touched files.
  • Change SPDX-FileCopyrightText ranges (e.g. "2023 - 2026") to a single starting year (e.g. "2023") in CMakeLists for dock, appruntimeitem, multitaskview, tray, and in dockiteminfo and trayitem headers.
panels/dock/CMakeLists.txt
panels/dock/appruntimeitem/CMakeLists.txt
panels/dock/multitaskview/CMakeLists.txt
panels/dock/tray/CMakeLists.txt
panels/dock/dockiteminfo.cpp
panels/dock/dockiteminfo.h
panels/dock/tray/trayitem.h

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

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:

  • In multitaskview.h the #include "../dockiteminfo.h" relative path makes the header fragile to directory moves; consider adjusting the include directories in CMake and using a project-level include path instead.
  • The same dockiteminfo.cpp/.h pair is now compiled into multiple libraries (dock, appruntimeitem, multitaskview, tray); if this sharing is intentional, consider extracting them into a small dedicated library to avoid duplicate compilation and potential ODR issues.
  • CMake usage for dockiteminfo.* mixes ${CMAKE_SOURCE_DIR}/panels/dock/... with relative paths like ../dockiteminfo.cpp; it would be clearer and less error-prone to standardize on a single style for referencing these sources.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `multitaskview.h` the `#include "../dockiteminfo.h"` relative path makes the header fragile to directory moves; consider adjusting the include directories in CMake and using a project-level include path instead.
- The same `dockiteminfo.cpp`/`.h` pair is now compiled into multiple libraries (dock, appruntimeitem, multitaskview, tray); if this sharing is intentional, consider extracting them into a small dedicated library to avoid duplicate compilation and potential ODR issues.
- CMake usage for `dockiteminfo.*` mixes `${CMAKE_SOURCE_DIR}/panels/dock/...` with relative paths like `../dockiteminfo.cpp`; it would be clearer and less error-prone to standardize on a single style for referencing these sources.

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.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这份 Git diff 展示了一次代码重构,主要目的是将 dockiteminfo 相关的文件从 frame 模块移动到 panels/dock 模块,并更新了相关的构建配置(CMakeLists.txt)和头文件包含路径。此外,版权声明的年份范围也被修正。

以下是对该变更的详细审查意见:

1. 语法逻辑与构建配置

  • 文件移动与包含路径:

    • 变更: dockiteminfo.hdockiteminfo.cppframe/ 移动到了 panels/dock/
    • 影响: 这导致原本依赖它的文件(如 multitaskview.h)中的包含路径 #include "dockiteminfo.h" 失效。
    • 修复: 代码中已将其修改为相对路径 #include "../dockiteminfo.h"
    • 审查意见: 使用相对路径 ../ 在 C++ 中是可行的,但会增加维护成本。如果未来目录结构再次调整,这些包含路径需要再次修改。
    • 改进建议: 建议在 panels/dock/CMakeLists.txt 中使用 target_include_directoriespanels/dock 目录添加到 dock-multitaskviewtrayitem 等目标(Targets)的公共接口包含目录中。这样,其他模块就可以直接使用 #include "dockiteminfo.h" 而不需要 ../,提高了代码的可移植性和可读性。
  • 源文件引用方式:

    • 变更: 在 panels/dock/appruntimeitem/CMakeLists.txt 中,使用了 ${CMAKE_SOURCE_DIR}/panels/dock/dockiteminfo.cpp 这种绝对路径变体来引用源文件。
    • 审查意见: 虽然这在功能上是正确的,但在 CMake 中直接列出源文件通常更倾向于使用相对路径或使用 GLOB(尽管 GLOB 有其缺点)。使用 ${CMAKE_SOURCE_DIR} 硬编码路径会使 CMakeLists.txt 文件变得冗长且不够灵活。
    • 改进建议: 既然文件已经在 panels/dock 目录下,如果构建系统允许,尽量保持引用路径的简洁性。或者,更好的做法是创建一个 dock-core 之类的静态库或接口库,专门包含 dockiteminfoconstants 等通用文件,然后让 appruntimeitemmultitaskviewtrayitem 链接这个库。这样可以避免在多个 CMakeLists.txt 中重复列出这些源文件(违反 DRY 原则)。

2. 代码质量

  • 模块化与依赖关系:
    • 现状: appruntimeitemmultitaskviewtrayitem 都直接包含并编译了 dockiteminfo.cpp
    • 审查意见: 这意味着 dockiteminfo.cpp 的目标代码会被编译进这三个不同的共享库中。这会导致代码体积膨胀(Code Duplication)。如果 dockiteminfo 中有全局静态变量,这三个库将各自拥有一份副本,可能导致状态不一致的严重 Bug。
    • 改进建议: 强烈建议将 dockiteminfo 编译成一个独立的共享库(或者如果 panels/dock 本身就是一个库,就放在那里),然后让其他模块通过链接(target_link_libraries)来使用它,而不是直接把源文件加到各自的编译列表中。

3. 代码性能

  • 二进制体积与加载时间:
    • 由于上述提到的源文件重复编译,最终生成的二进制文件体积会增加。
    • 如果这些是运行时动态加载的插件或共享库,重复的代码会增加磁盘 I/O 和内存占用(尽管现代操作系统的内存映射可能优化重复的物理页,但符号解析等开销依然存在)。

4. 代码安全

  • 版权声明:

    • 变更: 所有的 SPDX-FileCopyrightText2023 - 20262024 - 2026 修改为单一年份 20232024
    • 审查意见: 这是一个正确的修正。版权年份通常表示该文件首次发布的年份或修改发生的年份。使用未来的年份(如 2026)是不规范的,虽然不会导致编译错误,但在法律合规性上是不严谨的。
  • 头文件保护:

    • dockiteminfo.h 中使用了 #pragma once
    • 审查意见: 现代主流编译器(GCC, Clang, MSVC)都支持 #pragma once,且通常比标准的 #ifndef 宏守卫性能更好。但在极少数特殊文件系统或特定构建环境下(如符号链接、文件名大小写不敏感的文件系统映射),#pragma once 可能失效。
    • 改进建议: 虽然当前代码没有问题,但在跨平台极度严格的代码规范中,可能会建议同时使用 #pragma once 和传统的 #ifndef。不过对于当前项目,仅使用 #pragma once 是可以接受的。

总结建议

这次重构的主要逻辑是合理的(将文件移至更合适的目录),但在实现细节上有优化空间:

  1. 关键建议(最重要): 不要在 appruntimeitemmultitaskviewtrayitem 的 CMakeLists.txt 中直接添加 dockiteminfo.cpp 源文件。这会导致代码重复编译和潜在的状态不同步问题。应该将 dockiteminfo 编译进 panels/dock 的库中,或者创建一个新的共享库,供其他模块链接。
  2. 路径管理: 优化 CMake 的 include_directories,消除 C++ 代码中对 ../ 相对路径的依赖。
  3. 版权: 修正版权年份是正确的,请保持这种严谨性。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, wjyrich

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

@wjyrich

wjyrich commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Apr 16, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit c8251d0 into linuxdeepin:master Apr 16, 2026
8 of 12 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