Skip to content

Conversation

@LiHua000
Copy link
Contributor

The MIPS architecture uses Qt5 in V25, where the libdfm package has a different name compared to V20. This commit updates the dependency to reflect this change.

Log: build error

The MIPS architecture uses Qt5 in V25, where the libdfm package
has a different name compared to V20. This commit updates the
dependency to reflect this change.

Log: build error
@deepin-ci-robot
Copy link

deepin pr auto review

我来分析一下这个CMakeLists.txt的修改:

  1. 语法逻辑:
  • 代码语法正确,符合CMake语法规范
  • 条件判断逻辑清晰,使用了正确的变量比较方式
  1. 代码质量:
  • 优点:
    • 增加了对mips64架构的特殊处理,提高了代码的兼容性
    • 使用了清晰的变量命名(如PDF_LIB_VERSION、DTK_VERSION_MAJOR)
    • 添加了必要的find_package调用
  • 建议:
    • 可以添加注释说明为什么mips64架构需要特殊处理
    • 建议将mips64的判断逻辑封装成函数,提高代码可读性
  1. 代码性能:
  • 这部分代码主要在配置阶段执行,对运行时性能无影响
  • 建议将CMAKE_SYSTEM_PROCESSOR和DEEPIN_OS_VERSION的判断放在最前面,避免不必要的find_package调用
  1. 代码安全:
  • 代码中使用了REQUIRED标记,确保必要的依赖被正确找到
  • 建议添加对DEEPIN_OS_VERSION变量的存在性检查,防止未定义变量导致的错误

改进建议:

# 检查系统架构和版本
if(CMAKE_SYSTEM_PROCESSOR MATCHES "mips64")
    # 检查DEEPIN_OS_VERSION是否已定义
    if(NOT DEFINED DEEPIN_OS_VERSION)
        message(FATAL_ERROR "DEEPIN_OS_VERSION is not defined for mips64 architecture")
    endif()
    if (${DEEPIN_OS_VERSION} MATCHES "25")
        set(PDF_LIB_VERSION 5)
    endif()
endif()

# 根据Qt版本查找对应的DTK包
if (QT_VERSION_MAJOR MATCHES 6)
    find_package(Dtk6CMake REQUIRED)
    set(DTK_VERSION_MAJOR 6)
else()
    find_package(DtkCMake REQUIRED)
    set(DTK_VERSION_MAJOR "")
endif()

这些修改将:

  1. 增加错误检查,提高代码健壮性
  2. 优化执行顺序,减少不必要的包查找
  3. 添加注释,提高代码可维护性
  4. 将相关逻辑组织在一起,提高代码可读性

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, lzwind

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

@LiHua000
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 0b3e279 into linuxdeepin:master Nov 21, 2025
6 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