Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

Synchronize source files from linuxdeepin/dtklog.

Source-pull-request: linuxdeepin/dtklog#23

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

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

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

@github-actions
Copy link
Contributor

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

Synchronize source files from linuxdeepin/dtklog.

Source-pull-request: linuxdeepin/dtklog#23
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

我来对这段 CMakeLists.txt 的变更进行详细审查:

  1. 版本管理改进:
  • 优点:引入了更清晰的版本管理机制,使用 DTK_VERSION_MAJOR、DTK_VERSION_MINOR 和 DTK_VERSION_PATCH 分别管理主版本号、次版本号和修订号
  • 建议:DTK_VERSION 的格式 "0.${DTK_VERSION_MINOR}.${DTK_VERSION_PATCH}" 可能会造成混淆,建议直接使用完整的版本号格式
  1. 项目配置优化:
  • 优点:
    • 添加了 HOMEPAGE_URL,提供了项目主页信息
    • 使用 option() 命令替代了原来的 IF(BUILD_WITH_QT6) 判断,更符合 CMake 最佳实践
    • 引入了 DTK_NAME_SUFFIX 变量,使库命名更灵活
  1. 库版本控制:
  • 优点:SOVERSION 统一设置为 "0",有利于 ABI 兼容性管理
  • 建议:可以考虑使用更详细的版本控制策略,比如根据 DTK_VERSION_MAJOR 来设置 SOVERSION
  1. 依赖管理:
  • 优点:在 .pc 文件中添加了 "Requires: Qt@QT_VERSION_MAJOR@Core",明确声明了 Qt 依赖
  • 建议:可以在 CMakeLists.txt 中使用 find_package() 明确声明 Qt 依赖
  1. 代码组织改进:
  • 优点:
    • 变量命名更加一致(如将 DTK_FILE_VERSION 改为 FILE_VERSION)
    • 配置文件模板更新更加统一
  1. 潜在问题:
  • DTK5 选项默认为 ON,但没有文档说明 DTK5 和 DTK6 的具体区别
  • VERSION 和 SOVERSION 的设置可能需要更详细的文档说明其含义和影响
  1. 安全性考虑:
  • 使用了相对路径读取 VERSION 文件,建议添加文件存在性检查
  • 建议对版本号格式进行验证,防止非法输入

改进建议:

  1. 添加版本文件检查:
if(NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/VERSION")
    message(FATAL_ERROR "VERSION file not found")
endif()
  1. 添加版本号格式验证:
if(NOT FILE_VERSION MATCHES "^[0-9]+\\.[0-9]+\\.[0-9]+$")
    message(FATAL_ERROR "Invalid version format")
endif()
  1. 添加 DTK 版本说明文档注释:
# DTK5: Legacy version for Qt5-based applications
# DTK6: New version for Qt6-based applications
option(DTK5 "Build DTK5." ON)
  1. 考虑添加编译选项检查:
if(NOT BUILD_WITH_QT6 AND NOT DTK5)
    message(FATAL_ERROR "Invalid configuration: Qt5 build requires DTK5")
endif()

总体来说,这次变更改进了版本管理和项目配置,使代码更加规范和灵活,但还需要一些细节优化和文档补充。

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "  HOMEPAGE_URL \"https://github.com/linuxdeepin/dtklog\"",
            "line_number": 13,
            "rule": "S35",
            "reason": "Url link | 6fabe13a51"
        }
    ]
}

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.

2 participants