Skip to content

Conversation

@18202781743
Copy link
Contributor

  1. Added DtkTools dependency to dtkgui.cmake to support dconfig2cpp
    functionality
  2. Modified kernel.cmake to use dtk_add_config_to_cpp() for automatic
    generation of OrgDeepinDTKPreference class
  3. Removed manually maintained orgdeepindtkpreference.hpp file which was
    previously generated manually
  4. The change enables automatic generation of DConfig wrapper classes
    from JSON configuration files during build process

Log: Now using dconfig2cpp for automatic generation of DConfig types

Influence:

  1. Verify that the build system correctly generates
    orgdeepindtkpreference.hpp during compilation
  2. Test that the generated class works correctly with DConfig
    functionality
  3. Ensure all existing tests that use OrgDeepinDTKPreference continue
    to pass
  4. Check that the generated header file contains all required properties
    and methods
  5. Validate that DConfig value changes are properly propagated through
    the generated class

feat: 使用 dconfig2cpp 自动生成 dconfig 类型

  1. 在 dtkgui.cmake 中添加 DtkTools 依赖以支持 dconfig2cpp 功能
  2. 修改 kernel.cmake 使用 dtk_add_config_to_cpp() 自动生成
    OrgDeepinDTKPreference 类
  3. 删除之前手动维护的 orgdeepindtkpreference.hpp 文件
  4. 此更改使得在构建过程中能够从 JSON 配置文件自动生成 DConfig 包装类

Log: 现在使用 dconfig2cpp 自动生成 DConfig 类型

Influence:

  1. 验证构建系统在编译过程中正确生成 orgdeepindtkpreference.hpp
  2. 测试生成的类与 DConfig 功能正常工作
  3. 确保所有使用 OrgDeepinDTKPreference 的现有测试继续通过
  4. 检查生成的头文件包含所有必需的属性和方法
  5. 验证 DConfig 值更改通过生成的类正确传播

1. Added DtkTools dependency to dtkgui.cmake to support dconfig2cpp
functionality
2. Modified kernel.cmake to use dtk_add_config_to_cpp() for automatic
generation of OrgDeepinDTKPreference class
3. Removed manually maintained orgdeepindtkpreference.hpp file which was
previously generated manually
4. The change enables automatic generation of DConfig wrapper classes
from JSON configuration files during build process

Log: Now using dconfig2cpp for automatic generation of DConfig types

Influence:
1. Verify that the build system correctly generates
orgdeepindtkpreference.hpp during compilation
2. Test that the generated class works correctly with DConfig
functionality
3. Ensure all existing tests that use OrgDeepinDTKPreference continue
to pass
4. Check that the generated header file contains all required properties
and methods
5. Validate that DConfig value changes are properly propagated through
the generated class

feat: 使用 dconfig2cpp 自动生成 dconfig 类型

1. 在 dtkgui.cmake 中添加 DtkTools 依赖以支持 dconfig2cpp 功能
2. 修改 kernel.cmake 使用 dtk_add_config_to_cpp() 自动生成
OrgDeepinDTKPreference 类
3. 删除之前手动维护的 orgdeepindtkpreference.hpp 文件
4. 此更改使得在构建过程中能够从 JSON 配置文件自动生成 DConfig 包装类

Log: 现在使用 dconfig2cpp 自动生成 DConfig 类型

Influence:
1. 验证构建系统在编译过程中正确生成 orgdeepindtkpreference.hpp
2. 测试生成的类与 DConfig 功能正常工作
3. 确保所有使用 OrgDeepinDTKPreference 的现有测试继续通过
4. 检查生成的头文件包含所有必需的属性和方法
5. 验证 DConfig 值更改通过生成的类正确传播
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

Git Diff 代码审查报告

整体评估

这次修改主要涉及构建系统的调整,将原本手动维护的 orgdeepindtkpreference.hpp 文件替换为通过 dtk_add_config_to_cpp 函数自动生成的代码。这是一个很好的改进方向,提高了代码的可维护性和一致性。

详细审查

1. dtkgui.cmake 文件修改

-find_package(Dtk${DTK_NAME_SUFFIX} REQUIRED Core)
+find_package(Dtk${DTK_NAME_SUFFIX} REQUIRED Core Tools)

优点:

  • 添加了对 Tools 组件的依赖,这是必要的,因为新增的 dtk_add_config_to_cpp 函数很可能来自这个组件

建议:

  • 确保在项目文档中更新依赖说明,明确指出现在需要 Tools 组件

2. kernel.cmake 文件修改

set(kernel_SRC 
  ${KERNEL_HEADER}
  ${KERNEL_SOURCE}
-  ${CMAKE_CURRENT_LIST_DIR}/orgdeepindtkpreference.hpp
)
+
+dtk_add_config_to_cpp(kernel_SRC /usr/share/dsg/configs/org.deepin.dtk.preference.json
+  OUTPUT_FILE_NAME orgdeepindtkpreference.hpp
+  CLASS_NAME OrgDeepinDTKPreference
+)

优点:

  • 移除了手动维护的头文件,改用自动生成,减少了维护成本
  • 保持了相同的输出文件名和类名,确保了向后兼容性
  • 使用了专门的函数来处理配置到C++代码的转换

问题和建议:

  1. 硬编码路径问题

    • /usr/share/dsg/configs/org.deepin.dtk.preference.json 是硬编码的绝对路径
    • 建议使用CMake变量或查找路径,例如:
      set(DSG_CONFIG_DIR "${CMAKE_INSTALL_PREFIX}/share/dsg/configs")
      dtk_add_config_to_cpp(kernel_SRC 
          "${DSG_CONFIG_DIR}/org.deepin.dtk.preference.json"
          OUTPUT_FILE_NAME orgdeepindtkpreference.hpp
          CLASS_NAME OrgDeepinDTKPreference
      )
  2. 文件生成位置

    • 需要确认生成的文件位置是否在构建目录中,而不是源代码目录
    • 如果生成在构建目录中,需要确保该路径被正确添加到包含路径中
  3. 依赖关系

    • 应该添加对JSON配置文件的依赖,确保当配置文件更改时,头文件会被重新生成
    • 可以添加类似以下的代码:
      add_custom_command(
          OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/orgdeepindtkpreference.hpp
          COMMAND dtk_add_config_to_cpp(...)
          DEPENDS /usr/share/dsg/configs/org.deepin.dtk.preference.json
      )

3. orgdeepindtkpreference.hpp 文件删除

优点:

  • 删除了手动维护的文件,避免了重复代码
  • 生成的代码通常更一致且不易出错

注意事项:

  • 确保生成的代码与原来手动维护的代码功能完全一致
  • 特别注意检查生成的代码中的线程安全处理、信号槽连接等关键部分

代码质量改进建议

  1. 文档更新

    • 更新项目文档,说明现在使用自动生成的配置类
    • 添加关于如何修改配置文件的说明
  2. 构建系统改进

    • 考虑将JSON配置文件路径作为CMake选项,以便在不同环境中灵活配置
    • 添加对生成文件的验证步骤,确保生成的代码符合预期
  3. 错误处理

    • 添加对配置文件存在性和有效性的检查
    • 在生成失败时提供清晰的错误信息
  4. 跨平台考虑

    • 当前路径 /usr/share/dsg/configs/ 是Linux特有的
    • 如果需要跨平台支持,应该使用条件判断或变量来处理不同平台的路径

安全性考虑

  1. 输入验证

    • 确保 dtk_add_config_to_cpp 函数对输入的JSON配置文件进行验证
    • 防止恶意或格式错误的配置文件导致生成不安全的代码
  2. 路径安全

    • 避免使用用户可控的路径,防止路径遍历攻击
    • 确保配置文件只能从可信位置加载

性能考虑

  1. 构建时间

    • 自动生成的代码可能会增加构建时间
    • 考虑将生成的文件缓存起来,只在配置文件更改时重新生成
  2. 运行时性能

    • 确保生成的代码与原来的手动维护代码具有相同的性能特征
    • 特别注意线程安全和信号槽连接的效率

总结

这次修改总体上是积极的,将手动维护的配置类替换为自动生成的代码,提高了可维护性和一致性。主要需要关注的是硬编码路径的问题,应该将其改为使用CMake变量,以提高灵活性和跨平台兼容性。此外,应该添加适当的依赖关系和错误处理,确保构建系统的健壮性。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, zccrs

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

@18202781743
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 22, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 7ada9c4 into linuxdeepin:master Jan 22, 2026
19 of 21 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